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.

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

re-organise the compiler team #3599

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidtwco
Copy link
Member

Re-organise the compiler team:

  • Re-define and rename the tiers of membership
  • Change how team members and contributors are promoted
  • Document expectations of team members
  • Establish mechanism for scaling additional responsibilities that team members take on and recognising these contributions

Rendered text

@rust-lang/compiler @rust-lang/compiler-contributors

@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from 4bde403 to ac56ad4 Compare March 28, 2024 15:15
project member can raise concerns with an FCP, which will be considered by the
FCP reviewers.

To function effectively, it is recommended that there be 4 - 8 FCP
Copy link
Contributor

Choose a reason for hiding this comment

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

By what mechanism do we prevent growth beyond the desired range? Like, what if someone wants to be on FCP duty, but we already got 8 folk on the list?

Copy link
Member

Choose a reason for hiding this comment

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

Does a 4-8 limit actually matter in practice if all the members are responding "timely". We'd still have the problem of "FCPs span a wide variety of required knowledges and we dont want to encourage mindlessly checking a box because its not your area" but honestly maybe that's fine and if we are explicitly sayign to Not Do This we'd encourage growing the amount of documentation we have for areas so that people can more accurately evaluate fcps

Copy link
Contributor

Choose a reason for hiding this comment

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

"by signing up for FCPs you opt-in to start getting pinged after a week if you haven't reacted to the FCP. Contributors are encouraged to ping FCP folk if there are no more outstanding concerns."

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added some wording here - we don't want a hard upper limit, the goal is just to have a reasonable amount of diversity of opinion in the FCP list; make sure that everything gets a thorough look; and doesn't take too long. We want the FCP list to be populated by people who are committing to never mindlessly checking their box, they're on the list because they're interested in engaging with all of the FCPs we get. Team members who aren't on the FCP list and have an opinion can still register concerns and things like that to be heard.

Copy link
Member Author

Choose a reason for hiding this comment

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

"by signing up for FCPs you opt-in to start getting pinged after a week if you haven't reacted to the FCP. Contributors are encouraged to ping FCP folk if there are no more outstanding concerns."

GitHub hadn't shown me this comment when I was posting mine, I can change the wording to be closer to this if that's preferred.

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 specifying expectations for how frequently you're going to be pinged for reviews if you are an FCP reviewer is a good idea!

As David said, the upper limit is really just a suggestion, after ~8 people just collecting reviews from everyone in the group tends to take a while even when no major issues are raised. It's certainly possible that a larger group could also work if the FCP reviewers respond in a very timely manner.

Copy link
Member

Choose a reason for hiding this comment

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

What do we do if we feel that there are too many people on FCP? (and it gets too long) It feels trickly, it's not like we can just kick someone, there needs to be some process...

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the issue wouldn't be that there are too many people, but that some people on the list aren't reviewing promptly.

If we had twenty people on the list and everyone reviewed within a day, I think that would be okay, it's about making sure things move forward and setting expectations about promptness in FCP participation, rather than the number of people (unless maybe it meant that there was always some objection so nothing would progress - but that's a separate problem that we can deal with if we run into it).

@ehuss ehuss added the T-compiler Relevant to the compiler team, which will review and decide on the RFC. label Mar 28, 2024
Copy link
Member

@BoxyUwU BoxyUwU left a comment

Choose a reason for hiding this comment

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

I'm a big fan of these changes, thank you for working on this <3

Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

I like this. Thanks for working on this <3

I have some some comments and nitpicks though.

text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch 2 times, most recently from 917da30 to 1745d43 Compare April 2, 2024 12:43
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from 1745d43 to 4cc8a48 Compare April 2, 2024 13:08
@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from 4cc8a48 to 095140c Compare April 2, 2024 14:18
Copy link
Member

@lqd lqd 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!

text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch 2 times, most recently from 2dadeef to f4111c2 Compare April 2, 2024 15:36
@tmandry
Copy link
Member

tmandry commented Apr 3, 2024

This seems like a really positive change, thanks for the thought you've put into the RFC.

I have two questions that weren't directly answered:

  • What happens to the current groups? Are all current contributors going to become team members? Are all current team members going to become FCP reviewers?
  • Is the process for seconding and objecting to MCPs impacted at all?

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

really happy with this RFC ❤️ was a pleasure to read

@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from f4111c2 to 6a5782b Compare April 8, 2024 10:05
@davidtwco
Copy link
Member Author

  • What happens to the current groups? Are all current contributors going to become team members? Are all current team members going to become FCP reviewers?

Contributors would become team members and we'll go through the contributor list, see who would be eligible for maintainer and ask everyone if they want to be. After that, we'll work out who is involved in each activity.

  • Is the process for seconding and objecting to MCPs impacted at all?

It isn't, I've added some wording about this.

@davidtwco
Copy link
Member Author

I've also renamed "responsibilities" to "maintenance activities". I was never happy with "responsibilities" as the term and I think maintenance activities is better, but can change back if there is disagreement.

Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

This is amazing. Thank you @davidtwco (and everyone who has provided input).

@nnethercote
Copy link

This generally sounds good. My main thoughts are about how the various roles are recorded. I often look at the Compiler team and Compiler team contributors lists, which I view as the canonical source for the current memberships. The RFC doesn't mention those lists. What would they look like? Will there be a single place where I can see which people are signed up for which maintenance tasks? How will alumni be recorded?

@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from 6a5782b to 34d2ab5 Compare April 16, 2024 10:31
@davidtwco
Copy link
Member Author

What would they look like? Will there be a single place where I can see which people are signed up for which maintenance tasks? How will alumni be recorded?

These are all important questions - I've been thinking of them more as implementation details that we can work out after the RFC lands. Wesley and I will make sure that the team have a chance to give us feedback on however we end up representing this in the teams repo.

@wesleywiser
Copy link
Member

Will there be a single place where I can see which people are signed up for which maintenance tasks? How will alumni be recorded?

I would expect that the Compilers team page would look roughly similar to how it does currently except that there would be a clear delineation between individuals who are Maintainers or Members and there would also be indications as to what maintenance activities each individual participates in. I agree there is a lot of value in making that information easy to find!

There may be additional sources for some of that information so there will probably not be a single place you can find it. For example, I would expect that while there will probably be some kind of marker in rust-lang/team toml for the review rotation, that information will also be duplicated in triagebot.toml.

@wesleywiser
Copy link
Member

At this point, I believe all outstanding feedback has been responded to and there seems to be general consensus so:

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 17, 2024

Team member @wesleywiser 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.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Apr 17, 2024
@jackh726
Copy link
Member

Thinking about it: Is it worth distinguishing FCPs like this (i.e. FCPs about the team itself) from "typical" FCPs in regards to who is involved?

I actually don't know the right answer here! I think there are a few options, like "all Maintainers would be involved" or "team leads are responsible for ensuring consensus", and they have their own tradeoffs.

@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from 34d2ab5 to 7cb30ea Compare April 18, 2024 08:52
@davidtwco
Copy link
Member Author

Is it worth distinguishing FCPs like this (i.e. FCPs about the team itself) from "typical" FCPs in regards to who is involved?

I've added a little bit of wording saying that we have the option to include more people, such as the whole team or all maintainers, if it makes sense to do so, like on an FCP such as this.

Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

noticed typo :)

text/0000-compiler-team-trusted-contrib-maintainer.md Outdated Show resolved Hide resolved
Signed-off-by: David Wood <david@davidtw.co>
@davidtwco davidtwco force-pushed the compiler-team-trusted-contrib-maintainer branch from 7cb30ea to 0c74771 Compare April 23, 2024 10:04
of their pull requests (and pull requests they are reviewing). *rustc-perf*
permissions are useful for regular contributors as it is common to need to
request benchmarks from contributors with permissions. *rustc-perf* permissions
only make sense alongside *try* permissions.
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason to have this ability be separate from try? Is it purely to give further flexibility in case it is necessary; or are there additional security implications to @perfbot that need to be taken into the account that @bors try doesn't need to worry about?

Comment on lines +150 to +152
of contributions will be considered such as code contributions, helping with
issue triage and bisection, running meetings and creating minutes, documentation
contributions for rustc internals or the [Compiler Development Guide], etc.
Copy link
Member

Choose a reason for hiding this comment

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

I understand that it is possible to easily attach an objective criteria to the code contributions (number of merged pull requests, size of them) after the fact. What tools does GitHub (or our infrastructure in general) provide to list actions pertaining to triage, bisection, etc? Do we anticipate those to remain an action that a current member would need to consciously remind themselves to notice?

Choose a reason for hiding this comment

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

I can probably partially answer to this.

Non-code contributions to the project are basically invisible i.e. hard to quantify and produce stats for (unless, well, you look at the # of comments on GH and Zulip but they give a partial view1). We have tooling to facilitate this work (f.e. triagebot) and people like me are working to improve this tooling.

Do we anticipate those to remain an action that a current member would need to consciously remind themselves to notice?

IIUC your question you're asking if people doing non-code work know what to do and where to put their fingers on. The answer is yes and no because organizational work is relatively non-structured but we are looking into improving this.


Footnotes

  1. reminds me when Apple used to measure developer throughput by counting LOCs/week (link)

@inquisitivecrystal
Copy link
Contributor

inquisitivecrystal commented May 17, 2024

Before merging this, it would probably be worth changing the title of the RFC file to reflect the terminology currently used (rather than trusted contributor, which is from an earlier version). Actually, something based on the title of this PR, like compiler_team(_membership)_reorganization or reorganize_compiler_team(_membership), might be clearest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. T-compiler Relevant to the compiler team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet