Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

"Officialize" renaming of examples classes to addons #24579

Merged
merged 5 commits into from
Nov 1, 2022
Merged

"Officialize" renaming of examples classes to addons #24579

merged 5 commits into from
Nov 1, 2022

Conversation

LeviPesin
Copy link
Contributor

Related issue: #23406

Description

As we now have three/addons alias, and started to refer to examples classes as addons, isn't it the time to note that in documentation?

@LeviPesin LeviPesin mentioned this pull request Sep 3, 2022
Closed
7 tasks
Mugen87
Mugen87 previously approved these changes Sep 6, 2022
View reviewed changes
LeviPesin and others added 2 commits September 6, 2022 20:18
@Mugen87 Mugen87 added this to the r145 milestone Sep 6, 2022
@LeviPesin
Copy link
Contributor Author

I think fundamentals.html in new manual are missing the information about import maps...

@Mugen87 Mugen87 dismissed their stale review September 6, 2022 20:29

New changes were added since last review.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 7, 2022 •
edited
Loading

Renaming the doc pages will break a lot of URLs. Without implementing a redirect, I do not vote for doing this.

Besides, the PR got too complex. The first changes to the Installation guide were fine but now it's really hard to review everything. I suggest to revert and implement changes more incrementally.

@Mugen87 Mugen87 removed this from the r145 milestone Sep 7, 2022
@LeviPesin
Copy link
Contributor Author

I tried to git revert commits but that seems to broke everything...

@LeviPesin
Copy link
Contributor Author

Okay, I fixed it somehow 🙏

@LeviPesin
Copy link
Contributor Author

Besides, the PR got too complex. The first changes to the Installation guide were fine but now it's really hard to review everything. I suggest to revert and implement changes more incrementally.

Reverted those changes.

@looeee
Copy link
Collaborator

looeee commented Sep 8, 2022

@LeviPesin it might be better to open an issue for this first to see if there is support for the change, and also to figure out exactly what is required.

I'm personally in favor but as @Mugen87 points out it has to happen without breaking URLs. Which might make this more complicated than it first appears. On the other hand maybe it's ok just to change the docs and page titles and leave URLs as they are?

@Mugen87 Mugen87 added this to the r145 milestone Sep 9, 2022
@mrdoob mrdoob modified the milestones: r145, r146 Sep 29, 2022
@marcofugaro
Copy link
Contributor

Hey, I believe this can be merged 👍

@mrdoob mrdoob modified the milestones: r146, r147 Oct 27, 2022
@Mugen87 Mugen87 merged commit b71c5cf into mrdoob:dev Nov 1, 2022
@LeviPesin LeviPesin deleted the patch-2 branch November 1, 2022 09:27
@Mugen87 Mugen87 mentioned this pull request Nov 1, 2022
Merged
@LeviPesin LeviPesin mentioned this pull request Nov 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcofugaro marcofugaro marcofugaro left review comments

@Mugen87 Mugen87 Mugen87 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r147
Development

Successfully merging this pull request may close these issues.
5 participants
@LeviPesin @Mugen87 @looeee @marcofugaro @mrdoob