Skip to content

Add rustdoc team processes #852

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
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add rustdoc team processes #852

wants to merge 5 commits into from

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented May 21, 2025 •
edited by rustbot
Loading

cc @rust-lang/rustdoc

Rendered

fmease
fmease reviewed May 21, 2025
View reviewed changes
fmease
fmease reviewed May 21, 2025
View reviewed changes
fmease
fmease reviewed May 21, 2025
View reviewed changes
@ehuss
Copy link
Contributor

ehuss commented May 21, 2025

Can you also add the rustdoc team to triagebot.toml for this directory and give yourselves permissions in https://github.com/rust-lang/team/blob/HEAD/repos/rust-lang/rust-forge.toml?

@GuillaumeGomez GuillaumeGomez mentioned this pull request May 21, 2025
Merged
notriddle
notriddle requested changes May 21, 2025
View reviewed changes
aDotInTheVoid
aDotInTheVoid reviewed May 21, 2025
View reviewed changes
Copy link
Member

@aDotInTheVoid aDotInTheVoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I'm very happy to see this written down.

fmease
fmease reviewed May 21, 2025
View reviewed changes
@fmease fmease added T-rustdoc Team: Rustdoc needs-fcp This change is insta-stable, so needs a completed FCP to proceed. labels May 21, 2025
@fmease
Copy link
Member

fmease commented May 21, 2025 •
edited
Loading

(This PR will obviously need a T-rustdoc FCP to make it official once most things have been sufficiently fleshed out)

@fmease fmease added the S-waiting-on-team Status: Awaiting review/feedback/decision from relevant team(s) label May 21, 2025
Manishearth
Manishearth reviewed May 21, 2025
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall this seems pretty good!

@GuillaumeGomez
Copy link
Member Author

(This PR will obviously need a T-rustdoc FCP to make it official once most things have been sufficiently fleshed out)

Absolutely!

@GuillaumeGomez
Copy link
Member Author

I added the change in triagebot.toml as suggested by @ehuss.

I went through all comments.

@fmease: Hopefully I clarified that the rustdoc "groups" are not full-fledged teams and as such, to be part of them, you need to be a member of the rustdoc team.

fmease
fmease reviewed May 21, 2025
View reviewed changes
lolbinarycat
lolbinarycat reviewed May 21, 2025
View reviewed changes
Copy link
Contributor

@lolbinarycat lolbinarycat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few nitpicks

src/rustdoc/proposals-and-stabilization.md Outdated
Comment on lines 36 to 44
An FCP will be needed for any stabilization of small user-facing changes, like UI/UX changes, new
command-line arguments, new attributes, etc. However, if the change is considered too big/important,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should mention insta-stable GUI changes and the relaxed stability requirements thereof, since those aren't really a thing that exist anywhere else in rust project software.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rustdoc GUI is not considered stable (like the HTML/JS/CSS). We just try to limit its changes as much as possible. If it is a user-facing change, we want to go through an FCP to ensure that the reasons behind the change are valid and that majority agrees.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to clarify that UI/UX refers to the programmatic interface of rustdoc, not the web GUI (except for major changes).

lolbinarycat
lolbinarycat reviewed May 22, 2025
View reviewed changes
apiraino
apiraino requested changes May 22, 2025
View reviewed changes
Copy link
Contributor

@apiraino apiraino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a comment after a cursory read.

Great work @GuillaumeGomez !

@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc branch 2 times, most recently from 4da4ad9 to ab7ee18 Compare May 22, 2025 12:41
notriddle
notriddle reviewed May 22, 2025
View reviewed changes
@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc branch 2 times, most recently from e92f236 to cc605f3 Compare May 22, 2025 20:53
@GuillaumeGomez
Copy link
Member Author

@rfcbot fcp merge

@GuillaumeGomez

This comment was marked as outdated.

Sign in to view
@rfcbot
Copy link

rfcbot commented Jun 10, 2025 •
edited by camelid
Loading

Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Jun 10, 2025 •
edited
Loading

30 minutes latency, ok XD. Gonna edit my message haha.

jieyouxu
jieyouxu reviewed Jul 6, 2025
View reviewed changes
@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc branch 2 times, most recently from 718483f to a23c883 Compare July 7, 2025 10:19
@aDotInTheVoid aDotInTheVoid mentioned this pull request Jul 7, 2025
Merged
camelid
camelid approved these changes Jul 8, 2025
View reviewed changes
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great! I left a few comments about small clarifications, but the overall charter looks excellent.

src/rustdoc/proposals-and-stabilization.md Outdated
Comment on lines 36 to 44
An FCP will be needed for any stabilization of small user-facing changes, like UI/UX changes, new
command-line arguments, new attributes, etc. However, if the change is considered too big/important,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to clarify that UI/UX refers to the programmatic interface of rustdoc, not the web GUI (except for major changes).

@rfcbot
Copy link

rfcbot commented Jul 8, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @GuillaumeGomez, I wasn't able to add the final-comment-period label, please do so.

@camelid camelid added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Jul 8, 2025
@lolbinarycat
Copy link
Contributor

I think that we should have a section for what happens if you get assigned as a reviewer on a PR that you don't have the time or experience to review properly, such as if it's a large change to a sub-component you haven't worked with before. Having a policy for this would probably be quite helpful for any new team members.

@fmease
Copy link
Member

fmease commented Jul 9, 2025 •
edited
Loading

I think that we should have a section for what happens if you get assigned as a reviewer on a PR that you don't have the time or experience to review properly, such as if it's a large change to a sub-component you haven't worked with before.

This is already covered. See src/rustdoc/reviews.md:

Review Policy

The rustdoc team follows the same review policy as the compiler team. Take a look at
their chapter about it.

which delegates to compiler/reviews.md which in turn states (excerpts):

Basic Reviewing Requirements

There are a number of requirements that need to be met in order for reviewing to
be effective:

  • Reviewers must have a sufficient understanding of the code under review.
    […]
  • Reviewers must have a good idea on whether they are the right person to approve the change.
    […]

(see linked documents for more details)

@rfcbot
Copy link

rfcbot commented Jul 18, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

psst @GuillaumeGomez, I wasn't able to add the finished-final-comment-period label, please do so.

@fmease fmease added finished-final-comment-period The final comment period is finished for this RFC. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Jul 18, 2025
@GuillaumeGomez
Copy link
Member Author

Need approval from @apiraino and @notriddle since they requested changes to be able to merge. :)

apiraino
apiraino requested changes Jul 18, 2025
View reviewed changes
Copy link
Contributor

@apiraino apiraino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left 2 nits but otherwise LGTM 👍

apiraino
apiraino approved these changes Jul 18, 2025
View reviewed changes
Copy link
Contributor

@apiraino apiraino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:rocket: emoji for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Manishearth Manishearth Manishearth left review comments

@lolbinarycat lolbinarycat lolbinarycat left review comments

@jyn514 jyn514 jyn514 left review comments

@jieyouxu jieyouxu jieyouxu left review comments

@apiraino apiraino apiraino approved these changes

@fmease fmease fmease approved these changes

@aDotInTheVoid aDotInTheVoid aDotInTheVoid approved these changes

@camelid camelid camelid approved these changes

@notriddle notriddle Awaiting requested review from notriddle

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. finished-final-comment-period The final comment period is finished for this RFC. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-team Status: Awaiting review/feedback/decision from relevant team(s) T-rustdoc Team: Rustdoc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.