diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 76f1337f7..b377c1a02 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -43,22 +43,16 @@ - [Community](./community/README.md) - [State of Rust Survey FAQ](./community/survey-faq.md) - [Compiler](./compiler/README.md) - - [Cross Compilation](./compiler/cross-compilation/README.md) - - [Windows](./compiler/cross-compilation/windows.md) + - [Calendar](./compiler/calendar.md) - [Cross-team Collaboration](./compiler/cross-team-collaboration.md) - - [Review policy](./compiler/reviews.md) - - [So you want to add a new option to rustc?](./compiler/new_option.md) - - [Major Change Proposals](./compiler/mcp.md) + - [Meetings](./compiler/meetings.md) - [Membership](./compiler/membership.md) - - [Prioritization](./compiler/prioritization.md) - - [Procedure](./compiler/prioritization/procedure.md) - - [Priority Levels](./compiler/prioritization/priority-levels.md) - [Notification groups](./compiler/notification-groups.md) - - [Triage Meeting](./compiler/triage-meeting.md) - - [Steering Meeting](./compiler/steering-meeting.md) - - [Submitting a proposal](./compiler/steering-meeting/submit.md) - - [How to run the planning meeting](./compiler/steering-meeting/how-to-run-planning.md) - - [How to run a design meeting](./compiler/steering-meeting/how-to-run-design.md) + - [Resources](./compiler/resources.md) + - [Review Policy](./compiler/reviews.md) + - [Proposals, Approval and Stabilization](./compiler/proposals-and-stabilization.md) + - [Third-party and Out-of-tree Crates Policy](./compiler/third-party-out-of-tree.md) + - [Triage and Prioritization](./compiler/prioritization.md) - [crates.io](./crates-io/README.md) - [Crate removal](./crates-io/crate-removal.md) - [Database maintenance](./crates-io/db-maintenance.md) diff --git a/src/compiler/README.md b/src/compiler/README.md index 452d0ef63..e7130e39c 100644 --- a/src/compiler/README.md +++ b/src/compiler/README.md @@ -1,17 +1,27 @@ # Compiler -This section documents the Rust compiler itself, its APIs, and how to -contribute and provide bug fixes for the compiler. +Rust's compiler team are responsible for maintaining the Rust compiler, improving its performance +and considering the stabilization of compiler features. -### External Links -* The [Rustc Dev Guide] documents how the compiler works as well providing helpful - information to help get new contributors involved in the development. -* Rustc's [internal documentation]. -* The [Compiler team] website is the home for all of the compiler - team's planning. -* oli-obk's [FIXME page] lists all of the `FIXME` comments in the Rust compiler. +We use the Forge to document the team's processes, policies and working practices, if you'd like to +read about how the compiler works and instructions on how to set up a development environment, +you're looking for the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/). - -[Compiler team]: https://rust-lang.github.io/compiler-team/ -[FIXME page]: https://oli-obk.github.io/fixmeh/ -[Rustc Dev Guide]: https://rustc-dev-guide.rust-lang.org/ -[internal documentation]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ +- [Calendar](./calendar.md) + - *How do I subscribe to the compiler team's calendar?* +- [Cross-team Collaboration](./cross-team-collaboration.md) + - *How do I request the help of the compiler team?* +- [Meetings](./meetings.md) + - *What meetings do the compiler team run and how can I attend?* +- [Membership](./membership.md) + - *What is expected of compiler team members and how do I join?* +- [Resources](./resources.md) + - *What useful resources are available for contributors and team members?* +- [Review Policy](./reviews.md) + - *How do I make a contribution which is easy to review? How do I start reviewing as a team member?* +- [Proposals, Approval and Stabilization](./proposals-and-stabilization.md) + - *How do I propose a change to the compiler team? What approval is necessary for my change?* +- [Third-party and Out-of-tree Crates Policy](./third-party-out-of-tree.md) + - *When can I add third-party crates to the compiler? When can I create a out-of-tree crate for + the compiler?* +- [Triage and Prioritization](./prioritization.md) + - *How are compiler issues triaged and prioritized?* diff --git a/src/compiler/calendar.md b/src/compiler/calendar.md new file mode 100644 index 000000000..c4d64d271 --- /dev/null +++ b/src/compiler/calendar.md @@ -0,0 +1,39 @@ +# Calendar +All of the compiler team's calendars are available in the Rust project's +[`rust-lang/calendar`][calendar_repo] repository. + +## Adding new events +Any project member can submit a pull request to [add new events][add_event] or +[subcalendars][add_calendar], just assign the compiler team's leads as a reviewer for the pull +request. + +## Subscribing to the calendar +The team's calendar is [distributed as an `ics` file][ics] that can be imported into your preferred +calendar application. + +> You can copy the calendar link from below: +> +> `https://rust-lang.github.io/calendar/compiler.ics` +> +> Alternative links which do not include working groups are available on the +> [calendar repository][calendar_repo]. + +- Fastmail + - Open the Settings page. Go to "Calendars" on the left sidebar. Scroll down to the + "Subscriptions" section and paste the link to the [compiler team calendar][ics] into the + field and press "Subscribe to calendar". +- Google Calendar + - Press the "+" icon next to "Other Calendars" in the left sidebar. Select "From URL" and paste + the link to the [compiler team calendar][ics] into the field and press "Add calendar". +- Outlook Web + - Go to the Calendar using the icon on the left sidebar. Select "Add Calendar" on the left and + then "Subscribe from the web", paste the link to the [compiler team calendar][ics] into the + field and press "Import". + +If your preferred calendar application isn't listed above, feel free to submit a pull request to +this documentation to add instructions above. + +[add_calendar]: https://github.com/rust-lang/calendar?tab=readme-ov-file#how-do-i-add-a-calendar +[add_event]: https://github.com/rust-lang/calendar?tab=readme-ov-file#how-do-i-add-an-event +[calendar_repo]: https://github.com/rust-lang/calendar +[ics]: https://rust-lang.github.io/calendar/compiler.ics diff --git a/src/compiler/cross-compilation/README.md b/src/compiler/cross-compilation/README.md deleted file mode 100644 index 3bd00a657..000000000 --- a/src/compiler/cross-compilation/README.md +++ /dev/null @@ -1,2 +0,0 @@ -# Cross Compilation -This subsection documents cross compiling your code on one platform to another. diff --git a/src/compiler/cross-compilation/windows.md b/src/compiler/cross-compilation/windows.md deleted file mode 100644 index 6a9793710..000000000 --- a/src/compiler/cross-compilation/windows.md +++ /dev/null @@ -1,31 +0,0 @@ -# Windows - -1. Acquire LLD somehow. Either your distro provides it or you have to build it - from source. -2. You'll need an lld-link wrapper, which is just lld using the link flavor so - it accepts the same flags as link.exe. You may either have a binary called - lld-link, or you may have to write some sort of script to wrap lld. -3. If you want to be able to cross compile C/C++ as well, you will need to - obtain clang-cl, which is clang pretending to be cl. -4. You'll need libraries from an existing msvc installation on Windows to link - your Rust code against. You'll need the VC++ libraries from either VS 2015 or - VS 2017, and the system libraries from either the Windows 8.1 or Windows 10 - SDK. Here are some approximate paths which may vary depending on the exact - version you have installed. Copy them over to your non-windows machine. - * VS 2015: `C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\lib` - * VS 2017: `C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.24728\lib` - * Windows 10 SDK: `C:\Program Files (x86)\Windows Kits\10\Lib\10.0.14393.0` - * Windows 8.1 SDK: `C:\Program Files (x86)\Windows Kits\8.1\Lib\winv6.3` -5. If you want to cross compile C/C++ you'll also need headers. Replace `lib` in - the above paths with `include` to get the appropriate headers. -6. Set your LIB and INCLUDE environment variables to semicolon separated lists - of all the relevant directories for the correct architecture. -7. In your .cargo/config add `[target.x86_64-pc-windows-msvc] linker = "lld-link"` - or whatever your lld pretending to be link.exe is called. -8. For cross compiling C/C++, you'll need to get the gcc crate working - correctly. I never tested it to cross compile, I have no idea whether it will - even do anything sane. -9. Install the appropriate target using rustup and pass - `--target=x86_64-pc-windows-msvc` while building. Hopefully it works. If it - doesn't, well... I don't know. - diff --git a/src/compiler/mcp.md b/src/compiler/mcp.md deleted file mode 100644 index 64ea882a5..000000000 --- a/src/compiler/mcp.md +++ /dev/null @@ -1,190 +0,0 @@ -# Major Change Proposals - -Introduced in [RFC 2904], a "major change proposal" is a lightweight -form of RFC that the compiler team uses for architectural changes that -are not end-user facing. (It can also be used for small user-facing -changes like adding new compiler flags, though in that case we also -require an `rfcbot fcp` to get full approval from the team.) Larger -changes or modifications to the Rust language itself require a full -RFC (the latter fall under the lang team's purview). - -[RFC 2904]: https://github.com/rust-lang/rfcs/blob/master/text/2904-compiler-major-change-process.md - -## Motivation -[motivation]: #motivation - -As the compiler grows in complexity, it becomes harder and harder to track what's going on. We don't currently have a clear channel for people to signal their intention to make "major changes" that may impact other developers in a lightweight way (and potentially receive feedback). - -Our goal is to create a channel for signaling intentions that lies somewhere between opening a PR (and perhaps cc'ing others on that PR) and creating a compiler team design meeting proposal or RFC. - -## Goals - -Our goals with the MCP are as follows: - -* Encourage people making a major change to write at least a few paragraphs about what they plan to do. -* Ensure that folks in the compiler team are aware the change is happening and given a chance to respond. -* Ensure that every proposal has a "second", meaning some expert from the team who thinks it's a good idea. -* Ensure that major changes have an assigned and willing reviewer. -* Avoid the phenomenon of large, sweeping PRs landing "out of nowhere" onto someone's review queue. -* Avoid the phenomenon of PRs living in limbo because it's not clear what level of approval is required for them to land. - -## Major Change Proposals - -If you would like to make a [major change] to the compiler, the process is as follows: - -[major change]: #What-constitutes-a-major-change - -* Open a tracking issue on the [rust-lang/compiler-team] repo using the [major change template]. - * A Zulip topic in the stream `#t-compiler/major changes` will automatically be created for you by a bot. - * If concerns are raised, you may want to modify the proposal to address those concerns. - * Alternatively, you can submit a [design meeting proposal] to have a longer, focused discussion. -* To be accepted, a major change proposal needs three things: - * One or more **reviewers**, who commit to reviewing the work. This can be the person making the proposal, if they intend to mentor others. - * A **second**, a member of the compiler team who approves of the idea, but is not the one originating the proposal. - * A **final comment period** (a 10 day wait to give people time to comment). - * The FCP can be skipped if the change is easily reversed and/or further objections are considered unlikely. This often happens if there has been a lot of prior discussion, for example. -* Once the FCP completes, if there are no outstanding concerns, PRs can start to land. - * If those PRs make outward-facing changes that affect stable - code, then either the MCP or the PR(s) must be approved with a - `rfcbot fcp merge` comment. - -[major change template]: https://github.com/rust-lang/compiler-team/issues/new?assignees=&labels=major-change%2C+T-compiler&template=major_change.md&title=%28My+major+change+proposal%29 - -## Conditional acceptance - -Some major change proposals will be conditionally accepted. This indicates that we'd like to see the work land, but we'd like to re-evaluate the decision of whether to commit to the design after we've had time to gain experience. We should try to be clear about the things we'd like to evaluate, and ideally a timeline. - -## Deferred or not accepted - -Some proposals will not be accepted. Some of the possible reasons: - -* You may be asked to do some prototyping or experimentation before a final decision is reached -* The idea might be reasonable, but there may not be bandwidth to do the reviewing, or there may just be too many other things going on. -* The idea may be good, but it may be judged that the resulting code would be too complex to maintain, and not worth the benefits. -* There may be flaws in the idea or it may not sufficient benefit. - -[rust-lang/compiler-team]: https://github.com/rust-lang/compiler-team -[design meeting proposal]: https://forge.rust-lang.org/compiler/steering-meeting.html - -## What happens if someone opens a PR that seems like a major change *without* doing this process? - -The PR should be closed or marked as blocked, with a request to create -a major change proposal first. - -If the PR description already contains suitable text that could serve -as an MCP, then simply copy and paste that into an MCP issue. Using an -issue consistently helps to ensure that the tooling and process works -smoothly. - -## Can I work on code experimentally before a MCP is accepted? - -Of course! You are free to work on PRs or write code. But those PRs should be marked as experimental and they should not land, nor should anyone be expected to review them (unless folks want to). - -## What constitutes a major change? - -The rough intuition is "something that would require updates to the [rustc-dev-guide] or the [rustc book]". In other words: - -* Something that alters the architecture of some part(s) of the compiler, since this is what the rustc-dev-guide aims to document. -* A simple change that affects a lot of people, such as altering the names of very common types or changing coding conventions. -* Adding a compiler flag or other public facing changes, which should be documented (ultimately) in the rustc book. This is only appropriate for "minor" tweaks, however, and not major things that may impact a lot of users. (Also, public facing changes will require a full FCP before landing on stable, but an MCP can be a good way to propose the idea.) - -Note that, in some cases, the change may be deemed **too big** and a full FCP or RFC may be required to move forward. This could occur with significant public facing change or with sufficiently large changes to the architecture. The compiler team leads can make this call. - -Note that whether something is a major change proposal is not necessarily related to the number of lines of code that are affected. Renaming a method can affect a large number of lines, and even require edits to the rustc-dev-guide, but it may not be a major change. At the same time, changing names that are very broadly used could constitute a major change (for example, renaming from the `tcx` context in the compiler to something else would be a major change). - -[rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org -[rustc book]: https://doc.rust-lang.org/rustc/index.html - -## Public-facing changes require rfcbot fcp - -The MCP "seconding" process is only meant to be used to get agreement -on the technical architecture we plan to use. It is not sufficient to -stabilize new features or make public-facing changes like adding a `-C` -flag. For that, an `rfcbot fcp` is required (or perhaps an RFC, if the -change is large enough). - -For landing compiler flags in particular, a good approach is to start -with an MCP introducing a `-Z` flag and then "stabilize" the flag by -moving it to `-C` in a PR later (which would require `rfcbot fcp`). - -Major change proposals are not sufficient for language changes or -changes that affect cargo. - -## Steps to open a MCP - -* Open a tracking issue on the [rust-lang/compiler-team] repo using the - [major change template]. -* Create a Zulip topic in the stream `#t-compiler/major changes`: - * The topic should be named something like "modify the whiz-bang - component compiler-team#123", which describes the change and links - to the tracking issue. - * The stream will be used for people to ask questions or propose changes. - -## What kinds of comments should go on the tracking issue in compiler-team repo? - -Please direct technical conversation to the Zulip stream. - -The compiler-team repo issues are intended to be low traffic and used for procedural purposes. Note that to "second" a design or offer to review, you should be someone who is familiar with the code, typically but not necessarily a compiler team member. - -* Announcing that you "second" or approve of the design. -* Announcing that you would be able to review or mentor the work. -* Noting a concern that you don't want to be overlooked. -* Announcing that the proposal will be entering FCP or is accepted. - -## How does one register as reviewer, register approval, or raise an objection? - -These types of procedural comments can be left on the issue (it's also good to leave a message in Zulip). See the -previous section. To facilitate a machine parsable scanning of the concerns please use the following syntax to formally -register a concern: -``` -@rfcbot concern reason-for-concern - - -``` - -And the following syntax to lift a concern when resolved: -``` -@rfcbot resolve reason-for-concern -``` - -## Who decides whether a concern is unresolved? - -Usually the experts in the given area will reach a consensus here. But if there is some need for a "tie breaker" vote or judgment call, the compiler-team leads make the final call. - -## What are some examples of major changes from the past? - -Here are some examples of changes that were made in the past that would warrant the major change process: - -* overhauling the way we encode crate metadata -* merging the gcx, tcx arenas -* renaming a widely used, core abstraction, such as the `Ty` type -* introducing cargo pipelining -* adding a new `-C` flag that exposes some minor variant - -## What are some examples of things that are too big for the major change process? - -Here are some examples of changes that are too big for the major change process, or which at least would require auxiliary design meetings or a more fleshed out design before they can proceed: - -* introducing incremental or the query system -* introducing MIR or some new IR -* introducing parallel execution -* adding ThinLTO support - -## What are some examples of things that are too small for the major change process? - -Here are some examples of things that don't merit any MCP: - -* adding new information into metadata -* fixing an ICE or tweaking diagnostics -* renaming "less widely used" methods - -## When should Major Change Proposals be closed? - -Major Change Proposals can be closed: - -* by the author, if they have lost interest in pursuing it. -* by a team lead or expert, if there are strong objections from key - members of the team that don't look likely to be overcome. -* by folks doing triage, if there have been three months of - inactivity. In this case, people should feel free to re-open the - issue if they would like to "rejuvenate" it. diff --git a/src/compiler/meetings.md b/src/compiler/meetings.md new file mode 100644 index 000000000..2efd3c5d6 --- /dev/null +++ b/src/compiler/meetings.md @@ -0,0 +1,56 @@ +# Meetings +The compiler team host various regular meetings to keep on top of regular business necessary for +the running of the team and delivery of a high-quality compiler toolchain. + +## Triage Meeting +The weekly triage meeting takes place on Zulip: considering any backports, reviewing +performance triage reports and discussing nominated issues. Triage meetings are held in the +[`#t-compiler/meetings`][meetings_channel] on Zulip. You can find the up-to-date meeting times in +[the team calendar](./calendar.md). Anyone can attend and it is recommended that compiler team +members do. + +### Generating the triage meeting agenda +See [*Prioritization*](./prioritization.md) for documentation on generating the triage meeting +agenda. + +## Steering/Planning Meeting +The weekly steering/planning meeting also takes place on Zulip and is intended to host high-level +discussions. Steering/planning meetings operate on a repeating schedule: + +- **Week 1:** Planning meeting + - Select the topics for the next three meetings from the team's proposed meetings. +- **Week 2-4:** Steering meeting + - Discuss the planned topic. + +During planning meetings, the team lead running the meeting will attempt to identify topics which +are relevant for discussion. Some topics may require more investigation before a discussion or may +be out-of-date or more relevant to another team. Depending on the availabilities of the meeting +proposer and relevant team members, the meetings will then be scheduled into the steering meeting +slots of the following weeks. Not all meeting slots need to be scheduled. + +Meetings are proposed by opening issues on [the compiler team's repository][team_repo] using the +"meeting proposal" template. Steering meetings are good opportunities to discuss issues with the +wider team that require a decision/vibe check and would take longer than is typically available +when discussing nominated issues in a triage meeting. + +It is expected that the proposer of a steering meeting prepare a short and informal document +describing the topic and including all necessary context for the discussion, but this does not +need to be prepared until the day of the meeting, it is not necessary for the initial meeting +proposal. + +Any contributor can propose a meeting topic. Some examples of good steering meeting topics include: + +- Proposals which require feedback from the team + - i.e. to refactor subsystems of the compiler or create new out-of-tree dependencies +- In-depth review of large contributions + - i.e. the author describing and answering questions about their work +- Coordination between the team and the rest of the project + - i.e. reviewing proposed project goals +- Deciding on a policy for the team + - i.e. how to handle blockers from external dependencies + +Scheduled planning and steering meetings can be found on the [compiler team's +calendar](./calendar.md). + +[team_repo]: https://github.com/rust-lang/compiler-team +[meetings_channel]: https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings diff --git a/src/compiler/membership.md b/src/compiler/membership.md index d3e2eea4d..2685c682e 100644 --- a/src/compiler/membership.md +++ b/src/compiler/membership.md @@ -1,105 +1,86 @@ # Membership - This team discusses membership in the compiler team. There are currently two levels of membership: * members: regular contributors with r+ rights, bot privileges, and access to [infrastructure] -* maintainers: members who have committed themselves to invest in the quality of the compiler and health of the compiler team +* maintainers: members who have committed themselves to invest in the quality of the compiler and + health of the compiler team [infrastructure]: ../infra/index.html ## The path to membership - -People who are looking to contribute to the compiler typically start -in one of two ways. They may tackle "one off" issues, or they may get -involved in some kind of existing working group. They don't know much -about the compiler yet and have no particular privileges. They are -assigned to issues using the triagebot and (typically) work with a -mentor or mentoring instructions. - -## Compiler team member - -Once an individual has been contributing regularly for -some time, they can be promoted to the level of a **compiler team -member** (see the section on [how decisions are made][hdam] -below). This title indicates that they are someone who contributes -regularly. - -It is hard to define the precise conditions when such a promotion is -appropriate. Being promoted to member is not just a function of -checking various boxes. But the general sense is that someone is ready -when they have demonstrated three things: - -- "Staying power" -- the person should be contributing on a regular - basis in some way. This might for example mean that they have - completed a few projects. -- "Independence and familiarity" -- they should be acting somewhat - independently when taking on tasks, at least within the scope of the - working group. They should plausibly be able to mentor others on simple - PRs. -- "Cordiality" -- compiler team members will be part of the Rust organization and - are held to a higher standard with respect to the [Code of - Conduct][CoC]. They should not only obey the letter of the CoC but - also its spirit. +People who are looking to contribute to the compiler typically start in one of two ways. They may +tackle "one off" issues, or they may get involved in some kind of existing working group. They +don't know much about the compiler yet and have no particular privileges. They are assigned to +issues using the triagebot and (typically) work with a mentor or mentoring instructions. + +## Compiler team member +Once an individual has been contributing regularly for some time, they can be promoted to the +level of a **compiler team member** (see the section on [how decisions are made][hdam] below). +This title indicates that they are someone who contributes regularly. + +It is hard to define the precise conditions when such a promotion is appropriate. Being promoted +to member is not just a function of checking various boxes. But the general sense is that someone +is ready when they have demonstrated three things: + +- "Staying power" -- the person should be contributing on a regular basis in some way. This might + for example mean that they have completed a few projects. +- "Independence and familiarity" -- they should be acting somewhat independently when taking on + tasks, at least within the scope of the working group. They should plausibly be able to mentor + others on simple PRs. +- "Cordiality" -- compiler team members will be part of the Rust organization and are held to a + higher standard with respect to the [Code of Conduct][CoC]. They should not only obey the + letter of the CoC but also its spirit. [CoC]: https://www.rust-lang.org/policies/code-of-conduct Being promoted to member implies a number of privileges: -- Members have `r+` (approve a pull request) privileges and can do reviews - (they are expected to use those powers appropriately, as discussed - previously). They also have access to control perf/rustc-timer and other - similar bots. See the documentation for `bors` and `r+` +- Members have `r+` (approve a pull request) privileges and can do reviews (they are expected to + use those powers appropriately, as discussed previously). They also have access to control + perf/rustc-timer and other similar bots. See the documentation for `bors` and `r+` [here](https://rustc-dev-guide.rust-lang.org/compiler-team.html#team-membership). - Tip: some baseline rules around bors permissions are: don't do a `try` build - unless you have done a check for malicious code first and don't `r+` unless - you are reasonably confident that you can effectively review the code in - question. -- Compiler team members are members of the Rust organization so they can modify - labels and be assigned to issues. -- Members become a part of the rust-lang/compiler team on GitHub, - so that they receive pings when people are looking to address the - team as a whole. + Tip: some baseline rules around bors permissions are: don't do a `try` build unless you have + done a check for malicious code first and don't `r+` unless you are reasonably confident that + you can effectively review the code in question. +- Compiler team members are members of the Rust organization so they can modify labels and be + assigned to issues. +- Members become a part of the `rust-lang/compiler` team on GitHub, so that they receive pings + when people are looking to address the team as a whole. - Members are [listed] on the rust-lang.org web page. It also implies some obligations (in some cases, optional obligations): - Members will be asked if they wish to be added to the reviewer rotation. - Members may take part in various other [maintainer activities] to help the team. -- Members are held to a higher standard than ordinary folk when - it comes to the [Code of Conduct][CoC]. +- Members are held to a higher standard than ordinary folk when it comes to the [Code of + Conduct][CoC]. [listed]: https://www.rust-lang.org/governance/teams/compiler ## What it means to be a compiler member +Once you're a member of the compiler team, a number of events will happen: + +- You will gain access to a private Zulip stream, where internal discussions happen or ideas in + very draft state are shared. Come and say hello to your new team members! +- You will be subscribed and gain write access to a number of Github repositories. Check [this + GitHub page](https://github.com/orgs/rust-lang/teams/compiler/repositories) to see which + repositories you have now access to. Some of them are pretty quiet or obsolete, so don't worry + about all of them. -Once you're a member of the compiler team, a number of events will -happen: -- You will gain access to a private Zulip stream, where internal discussions -happen or ideas in very draft state are shared. Come and say hello to your new -team members! -- You will be subscribed and gain write access to a number of Github - repositories. Check [this GitHub - page](https://github.com/orgs/rust-lang/teams/compiler/repositories) - to see which repositories you have now access to. Some of them are pretty - quiet or obsolete, so don't worry about all of them. - - Tip: Github automatically adds you as subscriber to every repo you get write - permission too. You can disable this in the settings - ([here](https://github.com/settings/notifications)). + Tip: Github automatically adds you as subscriber to every repo you get write permission too. You + can disable this in the settings ([here](https://github.com/settings/notifications)). - You will also be subscribed to the `all@rust-lang.org` mailing list. See -[this file](https://github.com/rust-lang/team/blob/HEAD/teams/all.toml) to -check how subscriptions to mailing lists work. It's a very low-volume mailing -list (maybe a few emails per year), it's a way to communicate things to all -contributors. We will not send you spam from this address. + [this file](https://github.com/rust-lang/team/blob/HEAD/teams/all.toml) to check how subscriptions + to mailing lists work. It's a very low-volume mailing list (maybe a few emails per year), it's a + way to communicate things to all contributors. We will not send you spam from this address. ## Maintainers - After being a compiler team member for a year, members can request or be asked to become a -**compiler team maintainer**. This implies that they are not only a -regular contributor, but are actively helping to shape the direction -of the team or some part of the compiler (or multiple parts). +**compiler team maintainer**. This implies that they are not only a regular contributor, but are +actively helping to shape the direction of the team or some part of the compiler (or multiple +parts). - Compiler team maintainers are expected to participate in at least one [maintenance activities]. - Compiler team maintainers are identified with the "Maintainer" role on the rust-lang website. @@ -107,81 +88,65 @@ of the team or some part of the compiler (or multiple parts). ## How promotion decisions are made [hdam]: #how-promotion-decisions-are-made -After an individual has been contributing to the compiler for a while, -they may be nominated by an existing compiler team member or they may -ask the compiler team leads if their contribution history is sufficient -for team membership. +After an individual has been contributing to the compiler for a while, they may be nominated by an +existing compiler team member or they may ask the compiler team leads if their contribution history +is sufficient for team membership. -The compiler team leads will check with the rest of the compiler team -to see if there are concerns with extending a membership invitation to the -individual and after a week (barring no objections), an invitation will be extended. +The compiler team leads will check with the rest of the compiler team to see if there are concerns +with extending a membership invitation to the individual and after a week (barring no objections), +an invitation will be extended. -If the invitation is accepted by the individual, the compiler team leads -will update the [team] repository to reflect their new role. +If the invitation is accepted by the individual, the compiler team leads will update the [team] +repository to reflect their new role. [team]: https://github.com/rust-lang/team ## Not just code +It is worth emphasizing that becoming a member of the compiler team does not necessarily imply +writing PRs. There are a wide variety of tasks that need to be done to support the compiler and +which should make one eligible for membership. Such tasks would include organizing meetings, +participating in meetings, bisecting and triaging issues, writing documentation, working on the +rustc-dev-guide. -It is worth emphasizing that becoming a member of the -compiler team does not necessarily imply writing PRs. There are a wide -variety of tasks that need to be done to support the compiler and -which should make one eligible for membership. Such tasks would -include organizing meetings, participating in meetings, bisecting and -triaging issues, writing documentation, working on the rustc-dev-guide. -The most important criteria for elevation to compiler team member, -in particular, is **regular and consistent** participation. The most -important criteria for elevation to maintainer is **actively shaping the -direction of the team or compiler**. +The most important criteria for elevation to compiler team member, in particular, is +**regular and consistent** participation. The most important criteria for elevation to maintainer +is **actively shaping the direction of the team or compiler**. ## Alumni status +If at any time a compiler team member or maintainer wishes to take a break from participating, +they can opt to put themselves into alumni status. When in alumni status, they will be removed from +GitHub aliases and the like, so that they need not be bothered with pings and messages. They will +also not have r+ privileges. **Alumni members will however still remain members of the GitHub +org overall.** -If at any time a compiler team member or maintainer wishes to take a break -from participating, they can opt to put themselves into alumni status. -When in alumni status, they will be removed from Github aliases and -the like, so that they need not be bothered with pings and messages. -They will also not have r+ privileges. **Alumni members will however -still remain members of the GitHub org overall.** - -People in alumni status can ask to return to "active" status at any -time. This request would ordinarily be granted automatically barring -extraordinary circumstances. +People in alumni status can ask to return to "active" status at any time. This request would +ordinarily be granted automatically barring extraordinary circumstances. -People in alumni status are still members of the team at the level -they previously attained and they may publicly indicate that, though -they should indicate the time period for which they were active as -well. +People in alumni status are still members of the team at the level they previously attained and +they may publicly indicate that, though they should indicate the time period for which they were +active as well. ### Entering or leaving the Maintainer role +After a compiler team member has committed to actively maintaining the compiler by becoming a +Maintainer, they may wish to take a break from these ongoing responsibilities either temporarily +or indefinitely. In either case, the Maintainer can let the compiler team leads know or open a PR +themselves to the [team] repo, removing themselves from the Maintainer marker team and placing +themselves in the alumni list. -After a compiler team member has committed to actively maintaining the -compiler by becoming a Maintainer, they may wish to take a break from -these ongoing responsibilities either temporarily or indefinitely. -In either case, the Maintainer can let the compiler team leads know or -open a PR themselves to the [team] repo, removing themselves from the -Maintainer marker team and placing themselves in the alumni list. - -In the future, if the former Maintainer would like to resume maintenance -duties, they can request re-instatement from the compiler team leads. -This request would ordinarily be granted automatically barring -extraordinary circumstances. +In the future, if the former Maintainer would like to resume maintenance duties, they can request +re-instatement from the compiler team leads. This request would ordinarily be granted automatically +barring extraordinary circumstances. ### Compiler team alumni - -Likewise, if any member of the compiler team would like to take an -extended break from contribution and interaction with the team, -they can let the compiler team leads know or open a PR themselves +Likewise, if any member of the compiler team would like to take an extended break from contribution +and interaction with the team, they can let the compiler team leads know or open a PR themselves to the [team] repo, moving themselves to alumni status. -If an alumni member would like to resume compiler team membership -in the future, they can request re-instatement from the compiler team -leads and this will normally be granted. +If an alumni member would like to resume compiler team membership in the future, they can request +re-instatement from the compiler team leads and this will normally be granted. ### Automatic alumni status after 6 months of inactivity - -If a member or maintainer has been inactive in the compiler for 6 -months, then we will ask them if they would like to go to alumni -status. If they respond yes or do not respond, they can be placed on -alumni status. If they would prefer to remain active, that is also -fine, but they will get asked again periodically if they continue to -be inactive. +If a member or maintainer has been inactive in the compiler for 6 months, then we will ask them if +they would like to go to alumni status. If they respond yes or do not respond, they can be placed on +alumni status. If they would prefer to remain active, that is also fine, but they will get asked +again periodically if they continue to be inactive. diff --git a/src/compiler/new_option.md b/src/compiler/new_option.md deleted file mode 100644 index ade347ae9..000000000 --- a/src/compiler/new_option.md +++ /dev/null @@ -1,69 +0,0 @@ -# So you want to add a new (stable) option to rustc - -So you want to add a new command-line flag to rustc. What is the procedure? - -## Is this a perma-unstable option? - -The first question to ask yourself is: - -* Is this a "perma-unstable" option meant only for debugging rustc (e.g., `-Ztreat-err-as-bug`)? - -If so, you can just add it in a PR, no check-off is required beyond ordinary review. - -## Other options - -If this option is meant to be used by end-users or to be exposed on the stable channel, however, it represents a "public commitment" on the part of rustc that we will have to maintain, and hence there are a few more details to take care of. - -There are two main things to take care of, and they can proceed in either order, but both must be completed: - -* Proposal and check-off -* Implementation and documentation - -Finally, some options begin as unstable and only get stabilized over time, in which case you will also need: - -* Tracking issue and stabilization - -### Proposal and check-off - -The "proposal" part describes the motivation and design of the new option you wish to add. It doesn't necessarily have to be very long. It takes the form of a [Major Change Proposal][MCP]. - -[MCP]: https://forge.rust-lang.org/compiler/mcp.html - -The proposal should include the following: - -* **Motivation:** what is this flag used for? -* **Design:** What input does the flag take and what is its observable effect? -* **Implementation notes:** You don't have to talk about the implementation normally, but if there are any key things to note (i.e., it was very invasive to implement), you night note them here. -* **Precedent, links, and related material:** Are similar flags available on other compilers/linkers/tools, like clang or lld? -* **Alternatives, concerns, and key decisions:** Were there any alernatives considered? If so, why did you pick this design? - -Note that it is fine if you don't have any implementation notes, precedent, or alternatives to discuss. - -Also, one good approach to writing the MCP is basically to write the documentation you will have to write anyway to explain to users how the option works, and then add any additional notes on alternatives and so forth that are required. - -Once you've written up the proposal, you can [open a MCP](https://github.com/rust-lang/compiler-team/issues/new?assignees=&labels=major-change%2C+T-compiler&template=major_change.md&title=%28My+major+change+proposal%29) issue. But note that since this MCP is promoting a permanent change, a full compiler-team FCP is required, and not just a "second". This can be done by `@rfcbot fcp merge` by a team member. - -### Implementation, documentation - -Naturally your new option will also have to be implemented. You can implement the option and open up a PR. Often, this implementation work actually happens **before** the MCP is created, and that's fine -- we'll just ask you to open an MCP with the write-up. - -See the [Command-line Arguments] chapter in the rustc dev guide for guidelines on how to name and define a new argument. - -A few notes that are sometimes overlooked: - -* Many options begin as "unstable" options, either because they use `-Z` or because they require `-Zunstable-options` to use. -* You should document the option. Often this documentation can just be copied from the MCP text. Where you add this documentation depends on whether the option is available on stable Rust: - * If it is unstable, then document the option in the [Unstable Book](https://doc.rust-lang.org/nightly/unstable-book/index.html), whose sources are in [src/doc/unstable-book](https://github.com/rust-lang/rust/tree/master/src/doc/unstable-book). - * Once the option is stabilized, it should be documented in the [Rustc book](https://doc.rust-lang.org/rustc/index.html), whose sources as in [src/doc/rustc](https://github.com/rust-lang/rust/tree/master/src/doc/rustc). - -[Command-line Arguments]: https://rustc-dev-guide.rust-lang.org/cli.html - -## Stabilization and tracking issue - -Typically options begin as unstable, meaning that they are either used with `-Z` or require `-Zunstable-options`. - -Once the issue lands we should create a tracking issue that links to the MCP and where stabilization can be proposed. - -Stabilization generally proceeds when the option has a seen a bit of use and the implementation seems to be working as expected for its intended purpose. - -Remember that when stabilization occurs, documentation should be moved from the Unstable Book to the Rustc Book. diff --git a/src/compiler/notification-groups.md b/src/compiler/notification-groups.md index 8c30201ea..4819abe8d 100644 --- a/src/compiler/notification-groups.md +++ b/src/compiler/notification-groups.md @@ -1,42 +1,27 @@ # Notification groups - -The compiler team has a number of notification groups that we use to -ping people and draw their attention to issues. Notification groups -are setup so that anyone can join them if they want. +The compiler team has a number of notification groups that used to ping people and draw their +attention to issues. Notification groups are setup so that anyone can join them if they want. ## Creating a notification group +If you'd like to create a notification group, here are the steps. First, you want to get approval +from the compiler team: -If you'd like to create a notification group, here are the steps. -First, you want to get approval from the compiler team: - -* Propose the group by preparing a [Major Change Proposal][MCP]. If - your group is not analogous to some existing group, it is probably - a good idea to ping compiler team leads before-hand or as part of - the MCP. -* The MCP should specify what GitHub label will be associated with the - notification group. Often this is an existing label, such as - `O-Windows`. - -Once the MCP is accepted, here are the steps to actually create the group. -In some cases we include an example PR from some other group. - -* File a tracking issue in the [rust-lang/compiler-team] repository to collect - your progress. +* File a tracking issue in the [rust-lang/compiler-team] repository to collect your progress. * Create a PR against the [rust-lang/team] repository adding the notification - group. [Example PR.](https://github.com/rust-lang/team/pull/347) + group [(Example PR)](https://github.com/rust-lang/team/pull/347) * Configure the [rust-lang/rust] repository to accept triagebot commands - for this group. [Example PR.](https://github.com/rust-lang/rust/pull/72706) + for this group. [(Example PR.)))](https://github.com/rust-lang/rust/pull/72706) * Create a PR for the rustc-dev-guide amending [the notification group section](https://rustc-dev-guide.rust-lang.org/notification-groups/about.html) to mention your group. * Create a sample PR for the [rust-lang/team] repository showing how one can add oneself. This will be referenced by your blog post to show people how to - join. [Example PR.](https://github.com/rust-lang/team/pull/140) + join. [(Example PR)](https://github.com/rust-lang/team/pull/140) * Create a Zulip stream for the notification group. If you don't have the permission to do, you can ask on [#t-compiler/wg-meta]. * Write an announcement blog post for Inside Rust and open a PR against [blog.rust-lang.org](https://github.com/rust-lang/blog.rust-lang.org). - [Example PR.](https://github.com/rust-lang/blog.rust-lang.org/pull/615) + [(Example PR)](https://github.com/rust-lang/blog.rust-lang.org/pull/615) [rust-lang/compiler-team]: https://github.com/rust-lang/compiler-team [rust-lang/team]: https://github.com/rust-lang/team diff --git a/src/compiler/prioritization.md b/src/compiler/prioritization.md index f9c51ba06..2da50085c 100644 --- a/src/compiler/prioritization.md +++ b/src/compiler/prioritization.md @@ -1,3 +1,229 @@ # Prioritization +It is important that the compiler team can quickly identify priority issues, hence the establishment +of a prioritization process, described below. -This section documents the processes of the prioritization WG. +## General Process +1. Ascertain the current status of the issue +1. Try progressing the issue if possible (e.g. request updates from the issue author/reviewer) +1. Is there an MCVE for the issue already? +1. Check if it's a regression and label it accordingly (`regression-*` labels) +1. Figure out the area the issue belongs and label it accordingly (`A-*` labels) +1. [Ping notify groups](https://rustc-dev-guide.rust-lang.org/notification-groups/about.html) or + relevant teams +1. Assign if possible + +# Priority Levels +As the compiler team's resources are limited, the primary goal of prioritization is to identify the +most relevant issues to work on, so that the compiler team can focus on what matters the most. + +Issues relevant to prioritization are bugs and feature requests that are nominated for +prioritization, by adding the `I-prioritize` label as described below. + +## Labels +Labeling an issue as `I-prioritize` starts the prioritization process, which will end by +removing the `I-prioritize` label and appending one of the 4 labels we will discuss below: + +- `P-critical` +- `P-high` +- `P-medium` +- `P-low` + +Each of these labels defines a strategy the team will adopt regarding: + +- The amount of focus a given issue will receive +- How members of the community can get involved + +## P-critical +A `P-critical` is an issue potentially blocking a compiler release (i.e. highly recommended to be +solved before a new compiler release). These issues will be raised at the compiler team's triage +meeting on a weekly basis. + +Examples of things we typically judge to be “critical” bugs: + +- Regressions where code that used to compile no longer does + - Mitigating conditions that may lower priority: + - If the code should never have compiled in the first place (but if the regression affects a + large number of crates, this may indicate that we need a warning period) + - If the code in question is theoretical and considered unlikely to exist in the wild, or if + it only exists in small, unmaintained packages that are not widely used + - If a regression has been in stable for a release or two (either because we are still awaiting a + fix, or because the bug had laid dormant i.e. undetected), we typically lower the priority as + well, because by that time, if the users have not raised a ruckus about the regression, that + is a sign that it is inherently not a critical issue. + - e.g. [an issue that would have been `P-critical` but ended up being + `P-high`][critical_downgrade_example] +- Regressions where code still compiles but does something different than it used to do (dynamic + semantics have changed) + - Mitigating conditions that may lower priority: + - If code uses feature that is explicitly not specified (e.g. `std::vec::Vec` docs state order + in which it drops its elements is subject to change) +- Feature-gated features accessible without a feature gate + - Mitigating conditions that may lower priority: + - If the pattern is *very* unlikely +- Soundness holes with real-world implications + - Mitigating conditions that may lower priority: + - Soundness holes that are difficult to trigger + - Soundness holes that will not affect stable, e.g. if the hole makes use of a gated unstable + feature. +- Diagnostic regressions where the diagnostic is very common and the situation very confusing +- ICEs for common scenarios or code patterns + - Mitigating conditions that may lower priority: + - If the code that triggers the ICE also triggers compilation errors, and those errors are + emitted before the ICE + - If the code in question makes use of unstable features, particularly if the ICE requires a + feature gate + +A `P-critical` issue will receive the most attention. It must be assigned one or several people as +soon as possible, and the rest of the team should do their best to help them out if/when applicable. + +[critical_downgrade_example]: https://rust-lang.zulipchat.com/#narrow/stream/227806-t-compiler.2Fwg-prioritization/topic/pre-meeting.20triage.202020-04-09.20.2354818 + +## P-high +`P-high` issues are issues that need attention from the compiler team, but not to the point that +they need to be discussed at every meeting. They can be `P-critical` issues that have a mitigating +condition as defined above, or important issues that aren't deemed blockers. + +Because there are too many `P-high` issues to fit in every compiler meeting, they should rather be +handled asynchronously by the team's prioritization, in order to help them move forward. They can +still occasionally be brought up at meetings when it is deemed necessary. + +The effectiveness of the team's prioritization will be a direct consequence of the ability to draw +the line between `P-critical` and `P-high` issues. There shouldn't be too many `P-critical` issues +that compiler meetings become unmanageable, but critical issues shouldn't get lost in the list of +`P-high` issues. + +`P-high` issues are issues the teams will mostly work on. We want to make sure they're assigned, +and keep an eye on them. They are routinely reviewed in batches by the compiler team, deciding a +possible priority downgrade. + +## P-medium and P-low +`P-medium` refer to issues that aren't a priority for the team, and that will be resolved in the +long run. For example, issues that will be fixed after a specific feature has landed. They are +issues that the team could mentor someone interested in fixing. They will remain in this state +until someone complains, a community member fixes it, or it gets fixed by accident. + +`P-low` refer to issues issue that the compiler team doesn't plan to resolve, but are still worth +fixing. Nominate the issue if it's unclear and needs to be discussed. + +# Generating the triage meeting agenda +The triage meeting agenda is generated automatically using the prioritization efforts as input. +It is generated from a template available on [HackMD][template_hackmd] or [GitHub][template_github]. + +[template_hackmd]: https://hackmd.io/WQW0yzDDS16YvtBNurmj6A +[template_github]: https://github.com/rust-lang/compiler-team/blob/master/templates/T-compiler%20Meeting%20Agenda%20YYYY-MM-DD.md + +First, ensure that relevant issues are labelled as `T-compiler`.. + +- [Issues labeled with `I-prioritize`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+label%3AI-prioritize+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) +- [Pull requests nominated for the stable release channel backport](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Astable-nominated+-label%3Astable-accepted+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) +- [Pull requests nominated for the beta release channel backport](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Abeta-nominated+-label%3Abeta-accepted+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) +- [Issues labeled `I-compiler-nominated`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AI-nominated+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) (i.e. needing a T-compiler discussion) +- [Pull requests waiting on a team's feedback](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AS-waiting-on-team+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) +- [Issues classified with priority `P-high`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AP-high+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) + +..and that prioritization has been completed. Regressions labeled with `I-prioritize` are signaling +that a priority assessment is waiting. When this label is added to an issue, the `triagebot` creates +automatically a notification for @*WG-prioritization* members on [the +`#t-compiler/wg-prioritization/alerts` Zulip channel][prio_channel]. + +[prio_channel]: https://rust-lang.zulipchat.com/#narrow/stream/245100-t-compiler.2Fwg-prioritization.2Falerts + +To assign a priority, replace the `I-prioritize` label with one of `P-critical`, `P-high`, +`P-medium` or `P-low` and adding a succinct comment to link the Zulip discussion where the issue +prioritization occurred, example of a template for the comment: + +> WG-prioritization assigning priority ([Zulip discussion](#)). +> +> @rustbot label -I-prioritize +P-XXX + +Ideally, all [`T-compiler` issues with a `I-prioritize` label][issues_needing_prio] to have a +priority assigned, or strive to reach this goal: sometimes different factors are blocking issues +from being assigned a priority label, either because the report or the context is unclear or +because cannot be reproduced and an MCVE would help. Don't hesitate to ask for clarifications to +the issue reporter or ping the `ICEbreaker` team when an ICE ("Internal Compiler Errors") needs a +reduction (add a comment on the issue with `@rustbot ping icebreakers-cleanup-crew`) + +Review [stable][stable_regressions], [beta][beta_regressions] and [nightly][nightly_regressions] and +try to ensure they are assigned when possikle. + +[issues_needing_prio]: https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AT-compiler+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+label%3AI-prioritize +[stable_regressions]: https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3Aregression-from-stable-to-stable+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+-label%3AT-infra+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc +[beta_regressions]: https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3Aregression-from-stable-to-beta+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+-label%3AT-infra+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc +[nightly_regressions]: https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3Aregression-from-stable-to-nightly+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+-label%3AT-infra+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc + +The final step prior to generating the agenda is to accept any MCPs. Any MCPs that have had [the `final-comment-period` label][mcp_fcp] +for more than ten days can be accepted. Remove the `final-comment-period` label and add the `major-change-accepted` label and then +close the issue. + +[mcp_fcp]: https://github.com/rust-lang/compiler-team/issues?q=is%3Aissue+is%3Aopen+label%3Amajor-change+label%3Afinal-comment-period + +Finally, the meeting agenda can be generated. Clone and build [`triagebot`][triagebot] and set the +`GITHUB_TOKEN` environment variable with a GitHub API token. + +[triagebot]: https://github.com/rust-lang/triagebot + +Then generate the agenda with: + +``` +$ cargo run --bin prioritization-agenda +``` + +Copy the content into a new HackMD in the "Rust Lang Compiler Team" space. Copy the most recent +[performance triage logs][perf_triage_log] and remove anything that won't display well in Zulip + +[perf_triage_log]: https://github.com/rust-lang/rustc-perf/tree/master/triage#triage-logs + +Add additional manual details to the agenda: + +- Add summaries of stable/beta nominations (e.g. who nominated the backport and why) +- Add summaries of PRs waiting on the team (i.e. why are they waiting) +- Add initial impressions of `P-critical`/`P-high` bugs +- Add summaries of nominated issues (e.g. who the assignee is, why it was nominated, etc) +- Populate the oldest PRs waiting on review + - Use judgement to determine whether a ping is appropriate (e.g. if the pull request is an + experiment, it may not need a review; how long has it been since review activity; what do + recent comments say?) + +Two hours prior to the meeting, announce and share the completed agenda in the Zulip thread for the +upcoming meeting (creating it if it does not already exist): + +```text +Hi @*T-compiler/meeting*; the triage meeting will happen tomorrow in about 2 hours. +*WG-prioritization* has done pre-triage in #**t-compiler/wg-prioritization/alerts** +@*WG-prioritization* has prepared the [meeting agenda](link_to_hackmd_agenda) +``` + +It is always recommended to re-run the generator and copy any new details over to the agenda as +issue statuses on GitHub may have changed. + +After the meeting, there are a few closing tasks: + +- Lock the agenda on HackMD assigning write permissions to `Owners`. Download the Markdown file and + commit the agenda as minutes to the `compiler-team` repository. +- Remove the `to-announce` label from [MCPs][mcps], unless this label was added exactly during + the meeting (and therefore will be seen during the following meeting). +- Remove `to-announce` FCPs from [`rust-lang/rust` repo][announce], [`compiler-team` + repo][team_announce] and [forge repo][forge_announce]. Same disclaimer as before regarding changes + during the meeting. +- Accept or decline [`beta nominated`][beta_nominated] and [`stable nominated`][beta_nominated] + backports that have been accepted during the meeting. For more info check [`t-release` backporting + docs][release_backports] + - To accept a backport, add a `{beta,stable}-accepted` label and keep the `{beta,stable}-nominated` + label. Other automated procedures will process these pull requests, it's important to leave both + labels. Add a comment on Github linking the Zulip discussion. + - To decline a backport, simply remove `{beta,stable}-nominated` label. Add a comment on Github + explaining why the backport was declined and link the Zulip discussion. +- Remove [`I-compiler-nominated`][compiler_nominated] label from issues that were discussed. + Sometimes not all nominated issues are discussed (because of time constraints). In this example, + the `I-compiler-nominated` will stick until next meeting. Create a new agenda stub for the + following. + +[beta_nominated]: https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Abeta-nominated+-label%3Abeta-accepted +[stable_nominated]: https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Astable-nominated+-label%3Astable-accepted +[rust_announce]: https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Afinished-final-comment-period+label%3Ato-announce +[team_announce]: https://github.com/rust-lang/compiler-team/issues?q=is%3Aall+label%3Afinished-final-comment-period+label%3Ato-announce +[forge_announce]: https://github.com/rust-lang/rust-forge/issues?q=is%3Aall+label%3Afinished-final-comment-period+label%3Ato-announce +[fcps]: https://github.com/rust-lang/compiler-team/issues?q=is%3Aall+label%3Amajor-change+label%3Ato-announce +[mcps]: https://github.com/rust-lang/compiler-team/issues?q=is%3Aall+label%3Amajor-change+label%3Ato-announce +[relese_backports]: ../release/backporting.md +[compiler_nominated]: https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AI-nominated+label%3AT-compiler diff --git a/src/compiler/prioritization/priority-levels.md b/src/compiler/prioritization/priority-levels.md deleted file mode 100644 index c5dc1e9a5..000000000 --- a/src/compiler/prioritization/priority-levels.md +++ /dev/null @@ -1,73 +0,0 @@ -# Priority levels - -As the compiler team's resources are limited, the prioritization working group's main goal is to identify the most relevant issues to work on, so that the compiler team can focus on what matters the most. - -## Words used in this document: - -`issue` refers to bugs and feature requests that are nominated for prioritization, by flagging the `I-prioritize` label as described below. - -This document will define what each label means, and what strategy for each label will be used. - -## Labels - -Labeling an issue as `I-prioritize` starts the prioritization process, which will end by removing the `I-prioritize` label and appending one of the 4 labels we will discuss below: - -- P-critical -- P-high -- P-medium -- P-low - -Each of these labels defines a strategy the team will adopt regarding: - -- The amount of focus a given issue will receive -- How members of the community can get involved - -## P-critical - -A `P-critical` issue is a potentially blocker issue. - -The Working Group will keep track of these issues and will remind the compiler team on a weekly basis during the triage meeting. - -Examples of things we typically judge to be “critical” bugs: -- Regressions where code that used to compile no longer does - - Mitigating conditions that may lower priority: - - If the code should never have compiled in the first place (but if the regression affects a large number of crates, this may indicate that we need a warning period) - - If the code in question is theoretical and considered unlikely to exist in the wild, or if it only exists in small, unmaintained packages that are not widely used - - If a regression has been in stable for a release or two (either because we are still awaiting a fix, or because the bug had laid dormant i.e. undetected), we typically lower the priority as well, because by that time, if the users have not raised a ruckus about the regression, that is a sign that it is inherently not a critical issue. Eg: [an issue that would have been P-critical but ended up being P-high](https://rust-lang.zulipchat.com/#narrow/stream/227806-t-compiler.2Fwg-prioritization/topic/pre-meeting.20triage.202020-04-09.20.2354818) -- Regressions where code still compiles but does something different than it used to do (dynamic semantics have changed) - - Mitigating conditions that may lower priority: - - If code uses feature that is explicitly not specified (e.g. `std::vec::Vec` docs state order in which it drops its elements is subject to change) -- Feature-gated features accessible without a feature gate - - Mitigating conditions that may lower priority: - - If the pattern is VERY unlikely -- Soundness holes with real-world implications - - Mitigating conditions that may lower priority: - - Soundness holes that are difficult to trigger - - Soundness holes that will not affect stable, e.g. if the hole makes use of a gated unstable feature. -- Diagnostic regressions where the diagnostic is very common and the situation very confusing -- ICEs for common scenarios or code patterns - - Mitigating conditions that may lower priority: - - If the code that triggers the ICE also triggers compilation errors, and those errors are emitted before the ICE - - If the code in question makes use of unstable features, particularly if the ICE requires a feature gate - -A P-critical issue will receive the most attention. It must be assigned one or several people as soon as possible, and the rest of the team should do their best to help them out if/when applicable. - -## P-high - -`P-high` issues are issues that need attention from the compiler team, but not to the point that they need to be discussed at every meeting. -They can be `P-critical` issues that have a mitigating condition as defined above, or important issues that aren't deemed blockers. - -Because there are too many `P-high` issues to fit in every compiler meeting, they should rather be handled asynchronously by the Prioritization WG, in order to help them move forward. They can still occasionally be brought up at meetings when it is deemed necessary. - -The effectiveness of the Prioritization WG will be a direct consequence of our ability to draw the line between `P-critical` and `P-high` issues. There shouldn't be too many `P-critical` issues that compiler meetings become unmanageable, but critical issues shouldn't get lost in the list of P-high issues. - -P-high issues are issues the teams will mostly work on. We want to make sure they're assigned, and keep an eye on them. - -## P-medium and P-low - -`P-medium` refer to issues that aren't a priority for the team, and that will be resolved in the long run. Eg issues that will be fixed after a specific feature has landed. -They are issues we would mentor someone interested in fixing. -They will remain in this state until someone complains, a community member fixes it, or it gets fixed by accident. - - -`P-low` refer to issues issue that the compiler team doesn't plan to resolve, but are still worth fixing. diff --git a/src/compiler/prioritization/procedure.md b/src/compiler/prioritization/procedure.md deleted file mode 100644 index 095aea664..000000000 --- a/src/compiler/prioritization/procedure.md +++ /dev/null @@ -1,145 +0,0 @@ -# Prioritization WG - Procedure - -This document details the procedure the WG-prioritization follows to fill the agenda for the weekly meeting of `T-compiler`. -The working group focuses mainly on triaging `T-compiler` regressions, identifying possibly critical (and thus potential release blocker) issues and building the agenda for the weekly `T-compiler` meeting summarizing the main points to be discussed. - -## General issues review process - -- Check the status of the issue -- Try moving it forward if possible (ex. stimulate further comments from the issue author / reviewer) -- Ask for more info if it's needed -- Is there an MCVE for the issue already? -- Check if it's a regression and label it accordingly (`regression-*` labels) -- Figure out the area the issue belongs and label it accordingly (`A-*` labels) -- [Ping notify groups](https://rustc-dev-guide.rust-lang.org/notification-groups/about.html) or relevant teams -- Assign if possible -- Nominate the issue if it's unclear and needs to be discussed - -## Generating the T-compiler meeting's agenda - -The `T-compiler` agenda is generated from a template (available on [HackMD](https://hackmd.io/WQW0yzDDS16YvtBNurmj6A) or [Github](https://github.com/rust-lang/compiler-team/blob/master/templates/T-compiler%20Meeting%20Agenda%20YYYY-MM-DD.md)). We suggest working the following steps in this order: - -### Prepare agenda content - -#### 1. Add `T-compiler` labels where appropriate - -- [Issues labeled with `I-prioritize`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+label%3AI-prioritize+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) -- [Pull requests nominated for the stable release channel backport](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Astable-nominated+-label%3Astable-accepted+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) -- [Pull requests nominated for the beta release channel backport](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Abeta-nominated+-label%3Abeta-accepted+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) -- [Issues labeled `I-compiler-nominated`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AI-nominated+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) (i.e. needing a T-compiler discussion) -- [Pull requests waiting on a team's feedback](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AS-waiting-on-team+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) -- [Issues classified with priority `P-high`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AP-high+-label%3AT-compiler+-label%3AT-cargo+-label%3AT-core+-label%3AT-doc+-label%3AT-infra+-label%3AT-lang+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc+-label%3AA-rustdoc+-label%3AA-rustdoc-ui) - -#### 2. Assign a priority label to issues where needed - -Regressions labeled with `I-prioritize` are signaling that a priority assessment is waiting. When this label is added to an issue, the `triagebot` creates automatically a notification for @*WG-prioritization* members on [the Zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/245100-t-compiler.2Fwg-prioritization.2Falerts). - -To assign a priority, we replace the `I-prioritize` label with one of `P-critical`, `P-high`, `P-medium` or `P-low` and adding a succinct comment to link the Zulip discussion where the issue prioritization occurred, example of a template for the comment: - -> WG-prioritization assigning priority ([Zulip discussion](#)). -> -> @rustbot label -I-prioritize +P-XXX - -Ideally, we want all [`T-compiler` issues with a `I-prioritize` label](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3AT-compiler+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+label%3AI-prioritize) to have a priority assigned, or strive to reach this goal: sometimes different factors are blocking issues from being assigned a priority label, either because the report or the context is unclear or because cannot be reproduced and an MCVE would help. Don't hesitate to ask for clarifications to the issue reporter or ping the `ICEbreaker` team when an ICE ("Internal Compiler Errors") needs a reduction (add a comment on the issue with `@rustbot ping icebreakers-cleanup-crew`) - -Keep an eye also on regressions ([stable](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3Aregression-from-stable-to-stable+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+-label%3AT-infra+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc), [beta](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3Aregression-from-stable-to-beta+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+-label%3AT-infra+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc) and [nightly](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3Aregression-from-stable-to-nightly+-label%3AP-critical+-label%3AP-high+-label%3AP-medium+-label%3AP-low+-label%3AT-infra+-label%3AT-libs+-label%3AT-libs-api+-label%3AT-release+-label%3AT-rustdoc)), ideally they should an assignee. - -#### 3. Accept MCPs - -An MCP is a [Major Change Proposal](https://forge.rust-lang.org/compiler/mcp.html), in other words a change to the rust compiler that needs a bit more thought and discussion within the compiler team than a pull request. The life cycle of an MCP is described in the documentation. The relevant part for the WG-Prioritization is keeping an eye on them and accept all [MCPs that have been on `final-comment-period`](https://github.com/rust-lang/compiler-team/issues?q=is%3Aissue+is%3Aopen+label%3Amajor-change+label%3Afinal-comment-period) for 10 or more days. - -To accept an MCP, remove `final-comment-period` label, add `major-change-accepted` label and close the issue. A notification to the relevant Zulip topic ([in this stream](https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes)) will be automatically sent by the `triagebot`. - -### Generate the meeting's agenda - -Run triagebot's CLI to generate the agenda. You need to clone [https://github.com/rust-lang/triagebot](https://github.com/rust-lang/triagebot) (there is no official prepackaged release for this tool) and export two environment variables: `GITHUB_TOKEN` and optionally a `GOOGLE_API_KEY` to access a public Google calendar (if this env var is not found, meetings should be manually copy&pasted [from here](https://calendar.google.com/calendar/embed?src=6u5rrtce6lrtv07pfi3damgjus%40group.calendar.google.com)). - -To generate the meeting's agenda, run: - -``` -$ cargo run --bin prioritization-agenda -``` - -Copy the content of the generated agenda on HackMD. This will be our starting point. - -#### Add performance logs - -Paste the markdown file of this week [performance triage logs](https://github.com/rust-lang/rustc-perf/tree/master/triage#triage-logs) to the agenda and clean it up a little bit removing emojis (to make the text readable when pasted on Zulip). - -### Announce the meeting on Zulip - -About two hours before the scheduled meeting, create a new topic on the Zulip stream `#t-compiler/meetings` titled "\[weekly] YYYY-MM-DD" using the the following message template: - -```text -Hi @*T-compiler/meeting*; the triage meeting will happen tomorrow in about 2 hours. -*WG-prioritization* has done pre-triage in #**t-compiler/wg-prioritization/alerts** -@*WG-prioritization* has prepared the [meeting agenda](link_to_hackmd_agenda) - -Working group checkins for today: -- @**WG-foo** by @**person1** -- @**WG-bar** by @**person2** -``` - -Working Group checkins rotation are generated by a script [at this page](https://rust-lang.github.io/compiler-team/about/triage-meeting) (TODO: script is outdated and could probably be merged into the `triagebot` CLI code). - -Checkins about the progress of working groups are not mandatory but we rotate them all to be sure we don't miss on important progresses. - -### Add details to the Agenda - -#### 1. Summarize stable/beta nominations - -These are pull requests that the compiler team *might* want to backport to a release channel. Example a `stable-to-beta-regression` fix might want to be backported to the beta release channel. A `stable-to-stable-regression` fix particularly annoying might warrant a point release (i.e. release a `1.67.1` after a `1.67.0`). - -Follow the [General issues review process](#general-issues-review-process). - -#### 2. Summarize PRs waiting on team - -These are pull requests waiting on a discussion / decision from `T-compiler` (sometimes more than one team). - -Try to follow the [General issues review process](#general-issues-review-process). Explicitly nominate any issue that can be *quickly* resolved in a triage meeting. - -#### 3. Fill up the "Oldest PRs waiting for review" - -This is probably the less automatable part of the agenda (and likely the least fun). The `triagebot` will emit a list of 50 pull requests ordering them by least recent update. The idea is to issue mentions to assigned reviewers during the meeting ensuring that they stay on top of them. We usually try to keep the number of these mentions to around 5 for each meeting. - -There are two human factors here to keep in consideration: -- Pull requests reviewers are volunteers, we respect and appreciate their work. We don't want to remind them *too often* that there is a pile of pull requests waiting on them. Therefore we usually wait 2 or 3 weeks before reminding them about that pull requests. It seems like a long time to wait but let's not forget what contributors accomplish in the meanwhile! Anyway, we are trying to find ways to improve on these metrics. -- Contributors taking their time to submit a pull request deserve equally our appreciation so we try to not have them wait *too long* for a review or they will lose context about their work (or motivation to drive the contribution to completion). - -Striking a balance between these two diverging forces requires some empathy and "tribal knowledge" that comes with practice. Other factors can be blocking a pull request progress: -- The review is shared with another team (i.e. Team 1 says "OK", now waiting on Team 2) -- The alternating labels `S-waiting-on-review` and `S-waiting-on author` handling the life cycle of a pull request are not promptly applied. A pull request that is ready to be reviewed but it's *not* labeled `S-waiting-on-review` is idling for no purpose. - -#### 4. Add some context to `P-critical` and `P-high` regressions without an assignee - -Try to follow the [General issues review process](#general-issues-review-process). - -#### 5. Summarize I-compiler-nominated issues - -Issues labeled with `I-compiler-nominated` generally are nominated to specifically have the compiler team dedicate them a special slice of the meeting (generally towards the end). After the discussion, add a comment on Github linking the Zulip message where the discussion started (so everyone can read). `T-compiler` sometimes writes a summary of the discussion on the issue itself. - -Try to follow the [General issues review process](#general-issues-review-process): -- Check if an issue needs a discussion and add the label `I-compiler-nominated` -- When added to the agenda, add some context: - - Who the assignee is - - Is this an issue or a pull request: if it's an issue, does it have a pull request that fixes it? - - Why was it nominated - - Other important details - -### 6. Final review before the meeting - -Re-run the triagebot CLI script and update the agenda on HackMD with new data (if any). This is useful when there are last second changes affecting the agenda content. - -## Follow-ups after meeting - -The meeting is over! Time to cleanup a little bit. - -- Lock the agenda file on HackMD assigning write permissions to `Owners`. Download the markdown file and commit [it to this repository](https://github.com/rust-lang/compiler-team/issues?q=is%3Aall+label%3Amajor-change+label%3Ato-announce). - -- Remove the `to-announce` label from [MCPs](https://github.com/rust-lang/compiler-team/issues?q=is%3Aall+label%3Amajor-change+label%3Ato-announce), unless this label was added exactly during the meeting (and therefore will be seen during the following meeting). -- Remove `to-announce` FCPs from [rust repo](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Afinished-final-comment-period+label%3Ato-announce), [compiler-team repo](https://github.com/rust-lang/compiler-team/issues?q=is%3Aall+label%3Afinished-final-comment-period+label%3Ato-announce) and [forge repo](https://github.com/rust-lang/rust-forge/issues?q=is%3Aall+label%3Afinished-final-comment-period+label%3Ato-announce), same disclaimer as before. -- Accept or decline [`beta nominated`](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Abeta-nominated+-label%3Abeta-accepted) and [`stable nominated`](https://github.com/rust-lang/rust/issues?q=is%3Aall+label%3Astable-nominated+-label%3Astable-accepted) backports that have been accepted during the meeting. For more info check [T-release backporting docs](https://forge.rust-lang.org/release/backporting.html) - - To accept a backport, add a `{beta,stable}-accepted` label and keep the `{beta,stable}-nominated` label. Other automated procedures will process these pull requests, it's important to leave both labels. Add a comment on Github linking the Zulip discussion. - - To decline a backport, simply remove `{beta,stable}-nominated` label. Add a comment on Github explaining why the backport was declined and link the Zulip discussion. -- Remove [`I-compiler-nominated`](https://github.com/rust-lang/rust/issues?q=is%3Aopen+label%3AI-nominated+label%3AT-compiler) label from issues that were discussed. Sometimes not all nominated issues are discussed (because of time constraints). In this case the `I-compiler-nominated` will stick until next meeting. -- Create a new agenda stub for the following week using [our template](https://hackmd.io/WQW0yzDDS16YvtBNurmj6A) and post the link on Zulip, so it's available for people if they want to add content during the week. diff --git a/src/compiler/proposals-and-stabilization.md b/src/compiler/proposals-and-stabilization.md new file mode 100644 index 000000000..8bfded58d --- /dev/null +++ b/src/compiler/proposals-and-stabilization.md @@ -0,0 +1,336 @@ +# Proposals, Approvals and Stabilization +It is very common to need to gather feedback and approval when contributing to the compiler, either +for permission to proceed with an experiment or refactoring, or when stabilizing a feature. This +document aims to summarise the various processes that the compiler team has for making approval +decisions and when each should be used. + +## Approvals +There are three mechanisms that the team can use to approve a proposal (not all approval mechanisms +are suitable for each method of making a proposal - see below): + +- r+ + - A proposal is r+'d when it is approved to be merged. + - r+ can only be used to approve a PR. +- Seconding + - A proposal is second'ed when a team member formally endorses the proposal. It is intended that + seconding only occur once discussion has concluded and team members have had time to raise + concerns. Seconding tentatively accepts a proposal subject to a ten-day waiting period for + other team members to raise any concerns. + - Seconding can only be used to approve a MCP. +- FCP + - A final comment period will require sign-off from a majority of the compiler team to approve + a proposal and then a ten day waiting period. + - FCPs can be used to approve any form of proposal. + +## Proposals +There are three ways to propose a change to the compiler team. The appropriate choice depends on +the nature of the proposal, described below. + +- Request For Comments (RFC) + - RFCs are pull requests to the [`rust-lang/rfcs`][rfcs] repository and are a heavy-weight + proposal mechanism, reserved for significant changes. + - RFC proposals can only be approved by *FCPs*. +- Major Change Proposal (MCP) + - MCPs are issues in the [`rust-lang/compiler-team`][mcps] repository and are a medium-weight + proposal mechanism, suitable for most proposals. MCPs are recommended for written proposals + that are not end-user facing. + - Introduced in [RFC 2904][rfc_2904]. + - MCP proposals can be approved by *FCPs* or *Seconding*. +- Pull Request (PR) + - PRs are pull requests to the [`rust-lang/rust`][rust] repository and are a light-weight + proposal mechanism, suitable for most proposals. PRs are preferred when the proposal is + accompanied by a small patchset (such as stabilization of a compiler flag or addition of + a new target). + - PR proposals can be approved by *FCPs* or *r+*. + +[rfc_2904]: https://github.com/rust-lang/rfcs/blob/master/text/2904-compiler-major-change-process.md + +### How do I submit an MCP? + +* Open a tracking issue on the [rust-lang/compiler-team] repo using the [major change template]. + * A Zulip topic in the stream `#t-compiler/major changes` will automatically be created for you + by a bot. + * If concerns are raised, you may want to modify the proposal to address those concerns. + * Alternatively, you can submit a [design meeting proposal] to have a longer, focused discussion. +* To be accepted, a major change proposal needs three things: + * A **second**, a member of the compiler team who approves of the idea, but is not the one + originating the proposal. + * A **final comment period** (a 10 day wait to give people time to comment). + * The FCP can be skipped if the change is easily reversed and/or further objections are + considered unlikely. This often happens if there has been a lot of prior discussion, for + example. +* Once the FCP completes, if there are no outstanding concerns, contributions can begin. + * An earlier accepted MCP is not a substitute for any later necessary approvals. + +[rust-lang/compiler-team]: https://github.com/rust-lang/compiler-team +[design meeting proposal]: ./meetings.md#steeringplanning-meeting + +#### What kinds of comments should go on a MCP in the compiler-team repo? +Please direct technical conversation to the Zulip stream. + +The compiler-team repo issues are intended to be low traffic and used for procedural purposes. + +It is recommended that any team member who wishes to "second" a proposal be familiar with the +relevant code. Anyone can note concerns that shouldn't be overlooked. + +#### How does one second an MCP or raise an objection? +These types of procedural comments can be left on the issue (it's also good to leave a message in +Zulip). See the previous section. To facilitate a machine parsable scanning of the concerns +please use the following syntax to formally register a concern: + +``` +@rfcbot concern reason-for-concern + + +``` + +And the following syntax to lift a concern when resolved: + +``` +@rfcbot resolve reason-for-concern +``` + +MCPs can be seconded using: + +``` +@rfcbot second +``` + +##### Who decides whether a concern is unresolved? +Usually the experts in the given area will reach a consensus here, but if there is some +need for a "tie breaker" vote or judgment call, the compiler team leads make the final call. + +#### When should MCPs be closed? +MCPs can be closed: + +* by the author, if they have lost interest in pursuing it. +* by a team lead or expert, if there are strong objections from key members of the team that + don't look likely to be overcome. +* by folks doing triage, if there have been three months of inactivity. In this case, people + should feel free to re-open the issue if they would like to "rejuvenate" it. + +### What happens if someone makes a contribution that requires an approval and doesn't have one? +If the approval required for the contribution requires an MCP or an RFC, then the contribution +should be closed or marked as blocked, with a request to create an MCP or RFC first. If approval of +a PR is acceptable for the specific contribution (see below), then the approval process can begin. + +### Can I work on code experimentally before a approval is gained? +Of course! You are free to work on PRs or write code. But those PRs should be marked as +experimental and they should not land, nor should anyone be expected to review them (unless +folks want to). + +## What makes a good proposal? +A good proposal will address the following: + +* **Motivation:** Why is this proposal necessary? What problem does it solve? Why is that problem + important? +* **Design:** What are you proposing? +* **Implementation notes:** You don't have to talk about the implementation normally, but if there + are any key things to note (i.e., it was very invasive to implement), you might note them here. +* **Precedent, links, and related material:** Have there been similar proposals on other + compilers/linkers/tools, like `clang` or `lld`? +* **Alternatives, concerns, and key decisions:** Were there any alernatives considered? If so, why + did you pick this design? + +## What proposal/approval do I need? +This section aims to exhaustively detail which proposal and approval is necessary for any given +circumstance. + +### Internal + +- Creating [a notification group](./notification-groups.md) + - **Propose using:** PR + - **Approve using:** r+ + - If a team member finds the new group reasonable then they can merge the change adding the group. +- Significant internal refactorings/changes + - **Propose using:** MCP + - **Approve using:** Seconding + - Describe your proposed refactorings in detail in an MCP - optionally scheduling a steering + meeting if more focused discussion is necessary. Once discussion has concluded, a team member + may second the proposal +- Defining/changing small team policies + - **Propose using:** MCP + - **Approve using:** Seconding + - Examples of smaller policy changes where an MCP would be sufficient include our level of + support for case-insensitive filesystems or whether the team intend tracking issues to host + discussion +- Defining/changing large team policies + - **Propose using:** RFC + - **Approve using:** FCP + - Larger policy changes requiring an FCP include proposals to the team's structure and + membership criteria, etc + +### Compiler flags + +- Adding a compiler option for internal-use only (e.g. `-Ztreat-bug-as-err`) + - **Propose using:** PR + - **Approve using:** r+ + - If a team member finds the new option reasonable then they can merge the change adding the + option +- Adding a simple compiler option with intent to later stabilize + - **Propose using:** MCP + - **Approve using:** Seconding + - Simple options, such as exposing an uncontroversial option from LLVM, can be implemented and + merged with a seconded MCP and r+ approval from a reviewer. It will need a full FCP when it is + later stabilized +- Adding a complex compiler option with intent to later stabilize + - **Propose using:** RFC + - **Approve using:** FCP + - If the option is complicated and requires design considerations, then write and submit + a `t-compiler` RFC +- Removing internal-use only flags + - **Propose using:** MCP + - **Approve using:** Seconding + - Describe the rationale for removing the unstable implementation. Once discussion has concluded, + a team member may second the proposal +- Removing flags which were intended for eventual stabilization + - **Propose using:** MCP + - **Approve using:** Seconding + - Describe the rationale for removing the unstable implementation. Once discussion has concluded, + a team member may second the proposal +- Stabilizing a compiler option + - **Propose using:** PR + - **Approve using:** FCP + - Open a PR and follow the [stabilization guide][stabilization_guide]. The assigned reviewer will + check that the stabilization guide has been followed, review the code and start an FCP +- Reverting stabilization of a compiler option + - **Propose using:** PR + - **Approve using:** FCP + - Open a PR and follow the [stabilization guide][stabilization_guide]. The assigned reviewer will + check that the stabilization guide has been followed, review the code and start an FCP + +### Attributes + +- Adding a attribute for internal-use only (e.g. `rustc_attrs`) + - **Propose using:** PR + - **Approve using:** r+ + - If a team member finds the new attribute reasonable then they can merge the change adding the + attribute +- Adding a attribute with intent to later stabilize + - Follow the language team's process and have the implementation PR reviewed by a member of the + compiler team +- Removing internal-use only attributes + - **Propose using:** MCP + - **Approve using:** Seconding + - Describe the rationale for removing the unstable implementation. Once discussion has concluded, + a team member may second the proposal +- Removing attribute which were intended for eventual stabilization + - Follow the language team's process and have the removal PR reviewed by a member of the compiler + team +- Stabilizing an attribute + - Follow the language team's process and have the stabiization PR reviewed by a member of the + compiler team +- Reverting stabilization of an attribute + - Follow the language team's process and have the revert PR reviewed by a member of the + compiler team + +### Features + +- Adding experimental implementations of not-yet-proposed language features + - **Propose using:** MCP + - **Approve using:** Seconding + - With the approval of the language team (that they think the feature is worth experimentation), + then submit an RFC and if, after discussion has concluded, a compiler team member agrees that + the implementation is feasible and will not put undue burden on the maintainers of the compiler, + then they can second the MCP and implementation can proceed. + - This isn't necessary if the owner of the implementation is a member of the compiler team +- Stabilizing a language feature + - Follow the language team's process and have the stabiization PR reviewed by a member of the + compiler team +- Reverting stabilization of a language feature + - Follow the language team's process and have the revert PR reviewed by a member of the + compiler team + +### Targets + +- Proposing a new target + - **Propose using:** PR + - **Approve using:** r+ (compiler leads) + - Open a PR with the new target (w/ relevant documentation updates) and document adherence to the + [target tier policy][tier_policy] in the description. New targets must start as tier three + - New targets should be assigned to the compiler team co-leads to check for any licensing + concerns +- Promoting a target + - **Propose using:** PR + - **Approve using:** r+ (compiler leads) + - Open a PR with the new target and document adherence to the [target tier policy][tier_policy] + in the description + - New targets should be assigned to the compiler team co-leads to ensure that any demands on + the project infrastructure are considered and checked with relevant teams +- Demoting/removing a target + - **Propose using:** MCP + - **Approve using:** FCP + - Write an MCP describing why the target should be demoted/removed and once discussion has + concluded, an FCP can be started to approve the demotion/removal. +- Changing target baseline (e.g. minimum Darwin or Windows version bump) + - **Propose using:** MCP + - **Approve using:** FCP + - Write an MCP describing why the target should have a change of baseline and once discussion has + concluded, an FCP can be started to approve the change of baseline. +- Adding/removing target maintainers + - **Propose using:** PR + - **Approve using:** r+ + - Open a PR with the changes to the target documentation and obtain an r+ from the reviewer. +- Adding a target feature + - **Propose using:** PR + - **Approve using:** r+ + - Open a PR adding the target feature and obtain an r+ from the reviewer. +- Stabilizing a target feature + - **Propose using:** PR + - **Approve using:** FCP + - Open a PR stabilizing the target feature and once the reviewer is happy with the changes, + an FCP can be started + +### Lints, errors and warnings + +- Adding a new warning/error + - **Propose using:** PR + - **Approve using:** r+ + - Open a PR with the implementation and obtain an r+ from the reviewer +- Adding a new lint group + - Follow the language team's process and have the implementation PR reviewed by a member of the + compiler team +- Adding a new lint related to compiler features + - **Propose using:** MCP + - **Approve using:** FCP + - A lint concerning a detail that is otherwise the responsibility of the compiler team (such as + compiler flags) is the responsibility of the compiler to approve, rather than the language team. + - Write an MCP describing the lint and its justification and once discussion has concluded, an + FCP can be started to approve the new lint +- Adding a new future compatibility warning (FCW) related to compiler features + - **Propose using:** MCP + - **Approve using:** FCP + - A FCW concerning a detail that is otherwise the responsibility of the compiler team (such as + compiler flags) is the responsibility of the compiler to approve, rather than the language team. + - Write an MCP describing the FCW and its justification and once discussion has concluded, an + FCP can be started to approve the new FCW +- Changing default lint level of a lint related to compiler features + - **Propose using:** MCP + - **Approve using:** FCP + - A lint concerning a detail that is otherwise the responsibility of the compiler team (such as + compiler flags) is the responsibility of the compiler to approve, rather than the language team. + - Write an MCP describing the rationale for changing the default lint level and once discussion + has concluded, an FCP can be started to approve the new lint +- Adding a new lint related to language features + - Follow the language team's process and have the implementation PR reviewed by a member of the + compiler team +- Adding a new future compatibility warning (FCW) related to language features + - Follow the language team's process and have the implementation PR reviewed by a member of the + compiler team +- Changing default lint level of a lint related to language features + - Follow the language team's process and have the implementation PR reviewed by a member of the + compiler team + +### Licensing + +- Introducing a new dependency/license change/dependency bump + - **Propose using:** PR + - **Approve using:** r+ (compiler leads) + - Open a PR with the change affecting licensing and assign it to the team leads for review + +[stabilization_guide]: https://rustc-dev-guide.rust-lang.org/stabilization_guide.html +[tier_policy]: https://doc.rust-lang.org/rustc/target-tier-policy.html +[mcps]: https://github.com/rust-lang/compiler-team/issues?q=label%3Amajor-change +[rfcs]: https://github.com/rust-lang/rfcs +[rust]: https://github.com/rust-lang/rust +[compiler_lint_eg]: https://doc.rust-lang.org/rustc/lints/listing/deny-by-default.html#explicit-builtin-cfgs-in-flags diff --git a/src/compiler/resources.md b/src/compiler/resources.md new file mode 100644 index 000000000..61c0b016b --- /dev/null +++ b/src/compiler/resources.md @@ -0,0 +1,16 @@ +# Resources +There are various resources which are useful to contributors and team members. + +- [rustc Developer Guide][rustc] + - Documentation on compiler internals and setting up a developer environment. +- [rustc Generated Documentation][rustc] + - rustdoc output for the compiler sources +- [FIXMEH][fixmeh] + - An up-to-date list of all of the `// FIXME` comments in the compiler. + +If there are additional resources which would be useful to a contributor or compiler team member, +feel free to submit a PR to add it here. + +[rustc]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/index.html +[dev_guide]: https://rustc-dev-guide.rust-lang.org/ +[fixmeh]: https://oli-obk.github.io/fixmeh/ diff --git a/src/compiler/reviews.md b/src/compiler/reviews.md index 980f8a757..5acc92271 100644 --- a/src/compiler/reviews.md +++ b/src/compiler/reviews.md @@ -1,104 +1,81 @@ -Review Policy -============= +# Review Policy +This document describes our review policy for contributions to the Rust compiler. Its intended +audience are contributors and reviewers as well. -This document describes our review policy for contributions to the Rust -compiler. Its intended audience are contributors and reviewers as well. - -The purpose of this policy is to help contributors shape pull requests that -are easier to review - by clarifying the Rust project expectations - and for -reviewers as well - by having a handy list of common things to check. The -project will benefit if both parties have clear how to work together. +The purpose of this policy is to help contributors shape pull requests that are easier to review - +by clarifying the Rust project expectations - and for reviewers as well - by having a handy list of +common things to check. The project will benefit if both parties have clear how to work together. It is the purpose of code reviews to: -- **Reduce** the risk of introducing **bugs** and usability and performance - **regressions**. -- Keep our code **maintainable**: **readable**, **documented** and - **well-tested**. -- Ensure that changes are made with the big picture and **appropriate context in - mind**. This is particularly relevant for changes that seem harmless in - isolation but are problematic or undesirable in the larger context. - -Reviewing accomplishes this by bringing in another set of eyes to look at a -proposed change from a different perspective, which increases the chance of -catching mistakes early and uncovering potential blind spots in the reasoning -behind the change. +- **Reduce** the risk of introducing **bugs** and usability and performance **regressions**. +- Keep our code **maintainable**: **readable**, **documented** and **well-tested**. +- Ensure that changes are made with the big picture and **appropriate context in mind**. This is + particularly relevant for changes that seem harmless in isolation but are problematic or + undesirable in the larger context. +Reviewing accomplishes this by bringing in another set of eyes to look at a proposed change from a +different perspective, which increases the chance of catching mistakes early and uncovering +potential blind spots in the reasoning behind the change. ## 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. - - This is important to help spot non-obvious, unintentional side effects of a - given change. + - This is important to help spot non-obvious, unintentional side effects of a given change. - Pull request authors must provide: - 1. A concise high-level **description of the change** and (2) the - **rationale** behind it, unless the change is extremely trivial, like typo - fixes. - 2. For the rationale to be even more useful, authors are encouraged to list - potential **points of contention**, **compromises** that needed to be made, - **alternative approaches** that have been considered, **relevant - documentation, discussions and context**, etc. - - Reviewing code is difficult, and reviewers only have a **limited amount of - time** to do it. Jump-starting the review process by not making the - reviewer puzzle together the intention and context of a pull request will - not only speed things up but also improve the quality of the review. -- Reviewers must have a good idea on whether they are the **right person to - approve** the change. - - Knowledge of the code under review is an obvious but not the only criteria - for answering this question. + 1. A concise high-level **description of the change** and (2) the **rationale** behind it, + unless the change is extremely trivial, like typo fixes. + 2. For the rationale to be even more useful, authors are encouraged to list potential + **points of contention**, **compromises** that needed to be made, **alternative approaches** + that have been considered, **relevant documentation, discussions and context**, etc. + - Reviewing code is difficult, and reviewers only have a **limited amount of time** to do it. + Jump-starting the review process by not making the reviewer puzzle together the intention + and context of a pull request will not only speed things up but also improve the quality of + the review. +- Reviewers must have a good idea on whether they are the **right person to approve** the change. + - Knowledge of the code under review is an obvious but not the only criteria for answering + this question. - Procedure wise, the reviewer also needs to decide: - Can the reviewer make the decision alone? - - Does the PR needs to go through the [Major Change Process][MCP], or does - it need a T-compiler Final Comment Period sign-off (a buffer of time to - allow last comments or concerns before an official approval), or does it - need a full [Request For Comments (RFC)][rfc]? - - Does the PR need reviews and/or sign-off from other teams, particularly - T-lang? - - Can the changes break stable code or begin accepting new code that we do - not intend to? If the PR contains risks, is it sufficiently justified? - Does the changes need ecosystem impact evaluation through crater runs? - - Will the PR introduce significant perf changes? If there might be a perf - regression, is it justified? Does the PR need a perf run? - - Can the reviewer perform the review sufficiently thorough and in a timely - fashion? - - Is the reviewer impartial enough to provide a sufficiently unbiased - perspective? E.g. due to co-authorship (sufficiently significant changes - to the PR made by the reviewer) or other conflicts-of-interest? + - Does the PR need to go through [an approval process](./proposals-and-stabilization.md)? + - Does the PR need reviews and/or sign-off from other teams, particularly `t-lang`? + - Can the changes break stable code or begin accepting new code that we do not intend to? If + the PR contains risks, is it sufficiently justified? Does the changes need ecosystem impact + evaluation through crater runs? + - Will the PR introduce significant perf changes? If there might be a perf regression, is + it justified? Does the PR need a perf run? + - Can the reviewer perform the review sufficiently thorough and in a timely fashion? + - Is the reviewer impartial enough to provide a sufficiently unbiased perspective? e.g. due + to co-authorship (sufficiently significant changes to the PR made by the reviewer) or other + conflicts-of-interest? [rfc]: https://github.com/rust-lang/rfcs ## Reviewing Checklist - -The following list of questions will help reviewers and PR authors alike to -bring PRs into good shape and meet the above criteria: +The following list of questions will help reviewers and PR authors alike to bring PRs into good +shape and meet the above criteria: > #### Checklist for PR authors and reviewers -> * Does the PR message have -> * A concise **high-level description** of the changes? (*what* is being -> changed) -> * A clear **rationale** for doing them? (*why* is it being changed) -> * If non-trivial and if suitable, **how** the bug is fixed or **how** the -> change is implemented? -> * A list of potential **points of contention**? Alternatives? Trade-offs? -> Risks? -> * **Links to relevant issues**, RFCs, MCPs, etc? -> * Does the PR need a **regression test**? Does the PR have **sufficient test -> coverage**? -> * Does the change need to be covered by a **major change proposal ([MCP])**? Is -> it already covered? If there is already a MCP open, was it already accepted, -> or is the PR blocked on that? +> * Does the PR message have.. +> * ..a concise **high-level description** of the changes? (*what* is being changed) +> * ..a clear **rationale** for doing them? (*why* is it being changed) +> * ..if non-trivial and if suitable, **how** the bug is fixed or **how** the change is +> implemented? +> * ..a list of potential **points of contention**? alternatives? trade-offs? risks? +> * **.links to relevant issues**, RFCs, MCPs, etc? +> * Does the PR need a **regression test**? Does the PR have **sufficient test coverage**? +> * Does the change need to be covered by a **major change proposal ([MCP])**? Is it already +> covered? If there is already a MCP open, was it already accepted, or is the PR blocked on that? > * Does the PR need a **perf run**? -> * Does the PR need reviews and/or sign-offs from **other teams**? E.g. T-lang -> for lint expansions if the ecosystem impact is large or language changes. -> * Does the PR affect other teams in a non-trivial way? Should the affected -> teams get a heads-up? E.g. changes to rustfmt or rust-analyzer or subtrees. -> * Would someone trying to understand the PR in a year's time be able to -> quickly **reconstruct what's going on**? -> * Is the new code **properly documented**? Is existing documentation still -> up-to-date? +> * Does the PR need reviews and/or sign-offs from **other teams**? +> * e.g. `t-lang` for lint expansions if the ecosystem impact is large or language changes +> * Does the PR affect other teams in a non-trivial way? Should the affected teams get a heads-up? +> * e.g. changes to rustfmt or rust-analyzer or subtrees. +> * Would someone trying to understand the PR in a year's time be able to quickly **reconstruct +> what's going on**? +> * Is the new code **properly documented**? Is existing documentation still up-to-date? > * Does the changes in the PR need updates to the Reference or Edition Guide? > * Does the PR introduce a **regression** of any of the following: > * Error message quality @@ -111,7 +88,7 @@ bring PRs into good shape and meet the above criteria: > > #### Checklist for reviewers > * Am I the **right person to review** this PR: -> * Does the changes in this PR fall under T-compiler purview? +> * Does the changes in this PR fall under `t-compiler` purview? > * Do I understand the code well enough? > * Would I be able to spot non-obvious side effects? > * Would I be able to fix a bug introduced by this PR? @@ -127,217 +104,162 @@ bring PRs into good shape and meet the above criteria: ## Guidance for dealing with common situations - -In most cases common sense is enough for deciding how to apply this policy. -However, sometimes there are gray areas where it is not immediately clear how to -proceed. This section lists a few common cases together with guidance on how to -deal with them. +In most cases common sense is enough for deciding how to apply this policy. However, sometimes +there are gray areas where it is not immediately clear how to proceed. This section lists a few +common cases together with guidance on how to deal with them. ### I don't think I am a good fit for reviewing - what now? - -It is completely normal that you get (randomly) assigned a PR (via rustbot or -otherwise) but don't feel comfortable reviewing it. Here is what you can do, -depending on the concrete case: - -- If the change seems really big or contentious, consider asking for an MCP (see - below). The reviewer does not and should not be expected to just stomach a - large and/or significant PR. -- If you know just the right person for the review, assign them via `r? - @`. It's polite to leave a comment asking them if they can take - over -- but you don't have to make sure beforehand that they can actually do - it. -- If the change is not too complicated and you don't expect that another - randomly rolled compiler reviewer will also have trouble with the PR, you can - reroll a random compiler reviewer with `r? compiler`. -- If the change is complicated, or you expect randomly rolling another compiler - reviewer will just lead to multiple rerolls, you should open a thread in - `#t-compiler/private` to ask the rest of the team -- for someone who might be - able to review it, or even if the team is comfortable with accepting the - change at all. -- If the change is intended for another team, roll a reviewer from the relevant - team (example `r? compiler`) - -You can also always ask for help on the `#t-compiler` Zulip stream for finding a -reviewer. That being said, you are always welcome to do an initial review (to -the extent you are comfortable with) and then pass the PR on to the final -reviewer. This way the PR author will get helpful feedback sooner, subsequent -reviewers will have less work to do, and you might also improve your own -understanding of diverse areas in the compiler. - -### It is unclear if something constitutes a major change - -Deciding if something is a "major change" requiring an [MCP] is not always -straightforward. The official guidelines are [here][whats-a-major-change]. When -in doubt, err on side of treating something as a major change. You can also -nominate the PR for discussion in the compiler team's triage meeting by tagging -it `I-compiler-nominated`. If you nominate a PR please make sure to state a -**concrete question** for the compiler team to discuss, include **useful -context** if suitable. - -> **Example triage meeting nomination message** -> -> ```text -> Nominating for T-compiler triage meeting to determine if we want to make -> `-Z unstable-options` also accept value(s), e.g. `-Z unstable-options=values` to -> guard *unstable values* of *stable flags* like `-C opt-level=4`. -> -> Relevant discussion: . -> See also #123456. -> -> @rustbot label +I-compiler-nominated -> ``` +It is completely normal that you get (randomly) assigned a PR (via rustbot or otherwise) but don't +feel comfortable reviewing it. Here is what you can do, depending on the concrete case: + +- If the change seems really big or contentious, consider recommending the author go through the + [appropriate approval process](./proposals-and-stabilization.md). +- If you know just the right person for the review, assign them via `r? @`. It's polite + to leave a comment asking them if they can take over -- but you don't have to make sure + beforehand that they can actually do it. +- If the change is not too complicated and you don't expect that another randomly rolled compiler + reviewer will also have trouble with the PR, you can reroll a random compiler reviewer with + `r? compiler`. +- If the change is complicated, or you expect randomly rolling another compiler reviewer will just + lead to multiple rerolls, you should open a thread in `#t-compiler/private` to ask the rest of + the team -- for someone who might be able to review it, or even if the team is comfortable with + accepting the change at all. +- If the change is intended for another team, roll a reviewer from the relevant team (example: + `r? compiler`). + +You can also always ask for help on the `#t-compiler` Zulip stream for finding a reviewer. It is +recommended, to the extent you are comfortable with, to do an initial review before passing the PR +on to the final reviewer. This way the PR author will get helpful feedback sooner, subsequent +reviewers will have less work to do, and you might also improve your own understanding of diverse +areas in the compiler. + +### It is unclear if a contribution requires an approval decision +If you think a might contribution require broader team approval, check the [*Proposals, Approvals +and Stabilization*](./proposals-and-stabilization.md) documentation. If a contribution doesn't +match any of the examples in that documentation, open a thread in `#t-compiler/private` and ask. ### Discussion or rationale is too intransparent +Sometimes there are PRs that seem to be the result of some prior discussion, with no description +or rationale. They usually have a title like "Change X" and the only content of the PR message +is "r? @xyz". Even though the change might make sense and may even have been suggested by a +compiler team member, this is not good form. -Sometimes there are PRs that seem to be the result of some prior discussion, -with no description or rationale. They usually have a title like "Change X" and -the only content of the PR message is "r? @xyz". Even though the change might -make sense and may even have been suggested by a compiler team member, this is -not good form. +Contributors may stumble across the PR several year later during bisections only to find the PR +with absolutely zero context that was discussed offline or elsewhere, and the information is not +available to future contributors. This is not good for maintainability. Including relevant context +will very often help the PR author themselves in the future! -Contributors may stumble across the PR several year later during bisections only -to find the PR with absolutely zero context that was discussed offline or -elsewhere, and the information is not available to future contributors. This is -not good for maintainability. Including relevant context will very often help -the PR author themselves in the future! +The PR message should give a self-contained description of what is being changed, why it is +being changed and anything else that might be of interest. -The PR message should give a self-contained description of what is being -changed, why it is being changed and anything else that might be of interest. - -**Try to put yourself in the shoes of someone who, a few years down the road, -needs to fix a bug related to the code touched by the PR and needs to -reconstruct the rationale for the way things are.** +**Try to put yourself in the shoes of someone who, a few years down the road, needs to fix a bug +related to the code touched by the PR and needs to reconstruct the rationale for the way things +are.** ### Reviewer and PR author report to the same entity / work for the same employer +There is no rule that prevents two employees of the same company from reviewing each other's PRs. +We assume compiler team reviewers to act in good faith, and vest trust in team members to do so. -There is no rule that prevents two employees of the same company from reviewing -each other's PRs. We assume compiler team reviewers to act in good faith, and -vest trust in team members to do so. - -The concerns in such a case are no different than for any other two reviewers. -We expect the mechanisms and principles we articulated above to be respected by -ALL reviewers, whatever their employer. Does the PR concisely describe the -changes that are being made? Does it give a clear, transparent rationale for why -the changes make sense so that contributors down the line can follow the -reasoning and reconstruct what's going on? Have points of contention been -discussed and cleared up? Then you are in the clear. +The concerns in such a case are no different than for any other two reviewers. We expect the +mechanisms and principles we articulated above to be respected by *all* reviewers, whatever their +employer. Does the PR concisely describe the changes that are being made? Does it give a clear, +transparent rationale for why the changes make sense so that contributors down the line can follow +the reasoning and reconstruct what's going on? Have points of contention been discussed and +cleared up? Then you are in the clear. -If you are in doubt if something is contentious, give a heads up to -`@rust-lang/compiler` and ask for another opinion. If the proposed change is -large and/or potentially has a big impact, you can discuss in a `#t-compiler` -zulip topic, and/or create a [Major Change Proposal][MCP]. +If you are in doubt if something is contentious, give a heads up to `@rust-lang/compiler` and ask +for another opinion. If you think a might contribution require broader team approval, check +the [*Proposals, Approvals and Stabilization*](./proposals-and-stabilization.md) documentation. ### Reviewing and Mentoring - -In the course of mentoring someone through a PR it often happens that the -reviewer has ended up effectively co-writing the changes. This can be a tricky -case because the reviewer is effectively approving their own changes. There are -a number of considerations to take into account when deciding how to proceed: - -- If the general direction of the changes has already been approved as part of - an MCP and the concrete advice given during mentoring was only concerned with - resolving minor technical issues, then no further review is required. -- Similarly, if any contentious decisions have visibly been discussed and - resolved on the PR with other compiler team members and the rest of the - changes don't deviate from the general direction that has been agreed upon - then no further review is required either. -- If the PR was opened as a response to a concrete suggestion by the reviewer - (and the changes are not entirely trivial) then it is advisable that the final - review is done by someone else. However, the initial reviewer/mentor is - encouraged to help bring the PR into good shape before handing it off. - -In general, it is advisable to ask for a second opinion by someone knowledgable -in the field in such cases, just to increase the chance of uncovering oversights -and blindspots a mentor might have. +In the course of mentoring someone through a PR it often happens that the reviewer has ended up +effectively co-writing the changes. This can be a tricky case because the reviewer is effectively +approving their own changes. There are a number of considerations to take into account when deciding +how to proceed: + +- If the general direction of the changes has already been approved as part of an approval decision + and the concrete advice given during mentoring was only concerned with resolving minor technical + issues, then no further review is required. +- Similarly, if any contentious decisions have visibly been discussed and resolved on the PR with + other compiler team members and the rest of the changes don't deviate from the general direction + that has been agreed upon then no further review is required either. +- If the PR was opened as a response to a concrete suggestion by the reviewer (and the changes are + not entirely trivial) then it is advisable that the final review is done by someone else. However, + the initial reviewer/mentor is encouraged to help bring the PR into good shape before handing it + off. + +In general, it is advisable to ask for a second opinion by someone knowledgable in the field in +such cases, just to increase the chance of uncovering oversights and blindspots a mentor might have. ### Nobody understands the code that's being changed - -Sometimes there is a bug in some code that nobody understands anymore. The -original authors are unavailable and it is hard to gauge the implications of a -proposed fix. In such a case it is a good idea for reviewers to -`I-compiler-nominated` the PR (if they intend to stay the main reviewer) or +Sometimes there is a bug in some code that nobody understands anymore. The original authors are +unavailable and it is hard to gauge the implications of a proposed fix. In such a case it is a good +idea for reviewers to `I-compiler-nominated` the PR (if they intend to stay the main reviewer) or assign a compiler team lead to the issue and add the `S-waiting-on-team` label. -In both cases, the PR will be brought in the weekly triage meeting. It is also -especially valuable to gather and document as much information as possible about -the issue, such as a description of the problem being fixed, points of -unclarity, potential risks, alternatives that have been considered, et cetera. -It is also a good idea to open a tracking issue to document the lack of -understanding of such area, to document the specific questions, concerns and -bugs, and it can be resolved if compiler team members regain better -understanding. +In both cases, the PR will be brought in the weekly triage meeting. It is also especially valuable +to gather and document as much information as possible about the issue, such as a description of +the problem being fixed, points of unclarity, potential risks, alternatives that have been +considered, et cetera. It is also a good idea to open a tracking issue to document the lack of +understanding of such area, to document the specific questions, concerns and bugs, and it can be +resolved if compiler team members regain better understanding. -Reviewers should ask PR authors to add this kind of information as comments in -the code and/or to the PR message (which will become part of the git commit -history). +Reviewers should ask PR authors to add this kind of information as comments in the code and/or to +the PR message (which will become part of the git commit history). ### PR makes a change to support use of rustc internals for external projects - This will need to be determined on a case-by-case basis. -In general, we should allow changes making things public, cleaning up things or -making them more general, as long as the owners of the compiler region agree (so -just assign to them). +In general, we should allow changes making things public, cleaning up things or making them more +general, as long as the owners of the compiler region agree (so just assign to them). -As a concrete example: if someone is using the mir interpreter, and they want to -make something public, it is likely not a problem, but there are some functions -that are module- or crate-private on purpose, as they uphold invariants within -the mir interpreter. So basically, just assign such PRs to the relevant people -(usually they get pinged anyway due to having told highfive that they want to -get pinged on changes to these parts). +As a concrete example: if someone is using the mir interpreter, and they want to make something +public, it is likely not a problem, but there are some functions that are module- or crate-private +on purpose, as they uphold invariants within the MIR interpreter. So basically, just assign such +PRs to the relevant people (usually they get pinged anyway due to having told rustbot that they +want to get pinged on changes to these parts). -Require a doc comment on such APIs identifying which external consumers the API -concerns, and for what kinds of purpose. +Require a doc comment on such APIs identifying which external consumers the API concerns, and for +what kinds of purpose. -If this is possibly contentious, ask for an [MCP]. +If you think a might contribution require broader team approval, check the [*Proposals, Approvals +and Stabilization*](./proposals-and-stabilization.md) documentation. -Note that this can non-obviously bound supposedly-internal compiler APIs to -external consumers. Convey to the external consumers (that are not `rust-lang/` -projects) that we can offer the convenience so as long as it does not impose -significant maintenance burden on the compiler, e.g. gets in the way of -refactorings, and no hard stability guarantees are promised. +Note that this can non-obviously bound supposedly-internal compiler APIs to external consumers. +Convey to the external consumers (that are not `rust-lang/` projects) that we can offer the +convenience so as long as it does not impose significant maintenance burden on the compiler, e.g. +gets in the way of refactorings, and no hard stability guarantees are promised. ### The PR is very large and complicated - -Reviewers are **not** expected to stomach PRs that are very large and -complicated. It is expected from contributors to split their work to make a -review feasable, for example a series of more digestible PRs which are -individually more logically self-contained. In general, before submitting large -impact changes, it is expected the contributor to have discussed the design -previously with the relevant team(s) so it is contributor's duty to reference -such discussions. - -When in doubt, bring the PR to the attention of the team (through zulip threads -and/or nominate for compiler triage meeting), and the team can decide if: - -- The team can find suitable reviewers who can aid the PR author to break up the - large change into smaller logical PRs that are possible to review on their - own, but also in the context of the larger change. -- The team does not have the bandwidth, or team members are is not ready or - willing or able to accept the large change as-is. In such cases, the team - should make a decision to postpone or close, and clearly communicate the - decision to the PR author to explain the reasoning. It is very frustrating if - a PR stalls for many months only for it to be rejected anyway. - - -[MCP]: https://forge.rust-lang.org/compiler/mcp.html -[whats-a-major-change]: - https://forge.rust-lang.org/compiler/mcp.html#what-constitutes-a-major-change - +Reviewers are **not** expected to stomach PRs that are very large and complicated. It is expected +from contributors to split their work to make a review feasable, for example a series of more +digestible PRs which are individually more logically self-contained. In general, before submitting +large impact changes, it is expected the contributor to have discussed the design previously with +the relevant team(s) so it is contributor's duty to reference such discussions. + +When in doubt, bring the PR to the attention of the team (through zulip threads and/or nominate +for compiler triage meeting), and the team can decide if: + +- The team can find suitable reviewers who can aid the PR author to break up the large change into + smaller logical PRs that are possible to review on their own, but also in the context of the + larger change. +- The team does not have the bandwidth, or team members are is not ready or willing or able to + accept the large change as-is. In such cases, the team should make a decision to postpone or + close, and clearly communicate the decision to the PR author to explain the reasoning. It is + very frustrating if a PR stalls for many months only for it to be rejected anyway. ## Technical Aspects of Reviewing +Every PR that lands in the compiler and its associated crates must be reviewed by at least one +person who is knowledgeable with the code in question. -Every PR that lands in the compiler and its associated crates must be reviewed -by at least one person who is knowledgeable with the code in question. +When a PR is opened, you can request a reviewer by including `r? @username` in the PR description. +If you don't do so, rustbot will automatically assign someone from the pool of reviewer candidates, +determined by the files affected. -When a PR is opened, you can request a reviewer by including `r? @username` in -the PR description. If you don't do so, rustbot will automatically assign -someone from the pool of reviewer candidates, determined by the files affected. +It is common to leave a `r? @username` comment at some later point to request review from someone +else. This will also reassign the PR. -It is common to leave a `r? @username` comment at some later point to request -review from someone else. This will also reassign the PR. - -It is possible to request reviews from multiple reviewers, for example +It is possible to request reviews from multiple reviewers, for example: ```text Rolling both a T-compiler and T-bootstrap reviewer as this PR contains both @@ -348,76 +270,63 @@ r? bootstrap ``` ### bors +We never merge PRs directly. Instead, we use bors. A qualified reviewer with bors privileges +(e.g., a [compiler team member](./membership.md)) will leave a comment like `@bors r+`. This +indicates that they approve the PR. -We never merge PRs directly. Instead, we use bors. A qualified reviewer with -bors privileges (e.g., a [compiler team member](./membership.md)) will leave a -comment like `@bors r+`. This indicates that they approve the PR. - -People with bors privileges may also leave a `@bors r=username` command. This -indicates that the PR was already approved by `@username`. This is commonly done -after rebasing. +People with bors privileges may also leave a `@bors r=username` command. This indicates that the +PR was already approved by `@username`. This is commonly done after rebasing. Finally, in some cases, PRs can be "delegated" by writing `@bors delegate+` or -`@bors delegate=username`. This will allow the PR author or the delegated user -to approve the PR by issuing `@bors` commands like the ones above (but this -privilege is limited to the single PR). +`@bors delegate=username`. This will allow the PR author or the delegated user to approve the PR +by issuing `@bors` commands like the ones above (but this privilege is limited to the single PR). ### Reverts +If a merged PR is found to have caused a meaningful unanticipated regression, the best policy is +to revert it quickly and re-land it later once a fix and regression test are added. -If a merged PR is found to have caused a meaningful unanticipated regression, -the best policy is to revert it quickly and re-land it later once a fix and -regression test are added. - -A "meaningful regression" in this case is up to the judgment of the person -approving the revert. +A "meaningful regression" in this case is up to the judgment of the person approving the revert. Some criteria to consider if a revert is warranted: -- A bug in a stable or otherwise important feature that causes code to stop - compiling, changes runtime behavior, or triggers a (warn-by-default or higher) - lint incorrectly in real-world code. Especially if the bug is reachable - without any unstable feature gates. +- A bug in a stable or otherwise important feature that causes code to stop compiling, changes + runtime behavior, or triggers a (warn-by-default or higher) lint incorrectly in real-world code. + Especially if the bug is reachable without any unstable feature gates. - If a bug or change (incl. ICEs) is particularly easy to hit. - If a bug or change significantly degrades contributor experience. - If a test is flaky and unreliable. -When these criteria are in doubt, and especially if real-world code is affected, -revert the PR. This has three benefits: +When these criteria are in doubt, and especially if real-world code is affected, revert the PR. +This has three benefits: -1. It allows bleeding edge users (esp. nightly or beta) to continue to use and - report bugs on HEAD with a higher degree of certainty about where new bugs are - introduced. +1. It allows bleeding edge users (esp. nightly or beta) to continue to use and report bugs on HEAD + with a higher degree of certainty about where new bugs are introduced. 2. It takes pressure off the original PR author and the team, that no one is pressured to feel like they have to fix it *immediately*. 3. It might prevent the significant bug or regression from reaching another nightly/beta/stable build. -Before being reverted, a PR should be shown to cause a regression with a fairly -high degree of certainty (e.g. bisection on commits, or bisection on nightlies -with one or more compiler team members pointing to this PR, or it's simply -obvious to everyone involved). Only revert with lower certainty if the issue is -particularly critical or urgent to fix. +Before being reverted, a PR should be shown to cause a regression with a fairly high degree of +certainty (e.g. bisection on commits, or bisection on nightlies with one or more compiler team +members pointing to this PR, or it's simply obvious to everyone involved). Only revert with lower +certainty if the issue is particularly critical or urgent to fix. #### Creating reverts - -The easiest method for creating a revert is to use the "Revert" button on -Github. This appears next to the "bors merged commit abcd" message on a pull -request, and creates a new pull request. +The easiest method for creating a revert is to use the "Revert" button on Github. This appears +next to the "bors merged commit abcd" message on a pull request, and creates a new pull request. ![Location of the "Revert" button](revert-button.png) -Alternatively, a revert commit can be created using the git CLI and then -uploaded as a pull request: +Alternatively, a revert commit can be created using the git CLI and then uploaded as a pull request: ```terminal $ git revert -m 1 62d5bee ``` -Don't rely *only* on the default commit title and message created by git. -Instead, title the revert commit meaningfully, and link to the relevant PR that -introduced the regression. Link to the specific PR that is being fully or -partially reverted. Link to relevant issues and discussions. Retain the commit -hash being reverted. +Don't rely *only* on the default commit title and message created by git. Instead, title the revert +commit meaningfully, and link to the relevant PR that introduced the regression. Link to the +specific PR that is being fully or partially reverted. Link to relevant issues and discussions. +Retain the commit hash being reverted. > **Example revert commit title and message** > @@ -436,9 +345,8 @@ hash being reverted. > changes made to f415c07494b98e4559e4b13a9c5f867b0e6b2444. > ``` -It's polite to tag the author and reviewer of the original PR so they know -what's going on. You can use the following message template for the revert PR -description: +It's polite to tag the author and reviewer of the original PR so they know what's going on. +You can use the following message template for the revert PR description: ```text Reverts rust-lang/rust#123456 @@ -460,81 +368,66 @@ use cases working, and keep the compiler more stable for everyone. r? compiler ``` -Please include a temporary regression test in a separate commit to check that -the regression is actually addressed by the revert commit. In a reland, this -temporary regression test can be adapted or removed following improved test -coverage as suitable. +Please include a temporary regression test in a separate commit to check that the regression is +actually addressed by the revert commit. In a reland, this temporary regression test can be +adapted or removed following improved test coverage as suitable. -If you have r+ privileges, you can self-approve a revert **if** the revert is -clean and is unlikely to cause new regressions on its own, make sure the revert -is not a case of "the cure is worse than the poison". If non-trivial, please -wait for a review -- from the original reviewer or from another compiler -reviewer via `r? compiler`. You can ask in `#t-compiler` if the matter is more +If you have r+ privileges, you can self-approve a revert **if** the revert is clean and is unlikely +to cause new regressions on its own, make sure the revert is not a case of "the cure is worse +than the poison". If non-trivial, please wait for a review -- from the original reviewer or from +another compiler reviewer via `r? compiler`. You can ask in `#t-compiler` if the matter is more urgent. -Generally speaking, reverts should have elevated priority and match the `rollup` -status of the PR they are reverting. If a non-rollup PR is shown to have no -impact on performance, it can be marked `rollup=always`. The revert author can -coordinate with contributors authoring rollups to reschedule rollups or -interleave the revert PR between rollups if suitable. +Generally speaking, reverts should have elevated priority and match the `rollup` status of the PR +they are reverting. If a non-rollup PR is shown to have no impact on performance, it can be marked +`rollup=always`. The revert author can coordinate with contributors authoring rollups to reschedule +rollups or interleave the revert PR between rollups if suitable. #### Forward fixes - -Often it is tempting to address a regression by posting a follow-up PR that, -rather than reverting the regressing PR, instead augments the original in small -ways without reverting its changes overall. However, if real-world users have -reported being affected, this practice is strongly discouraged unless one of the -following is true: +Often it is tempting to address a regression by posting a follow-up PR that, rather than reverting +the regressing PR, instead augments the original in small ways without reverting its changes +overall. However, if real-world users have reported being affected, this practice is strongly +discouraged unless one of the following is true: * A high-confidence fix is already in the bors queue. -* The regression has made it to a release branch (beta or stable) and a - [backport] is needed. Often the "smallest possible change" is desired for a - backport (so that the fix doesn't introduce *new* regressions). The offending - PR may or may not still be reverted on the main branch; this is left to the - discretion of someone who can `r+` it. +* The regression has made it to a release branch (beta or stable) and a [backport] is needed. Often + the "smallest possible change" is desired for a backport (so that the fix doesn't introduce *new* + regressions). The offending PR may or may not still be reverted on the main branch; this is left + to the discretion of someone who can `r+` it. [backport]: ../release/backporting.md -While it can feel like a significant step backward to have your PR reverted, in -most cases it is much easier to reland the PR once a fix can be confirmed. -Allowing a revert to land takes pressure off of you and your reviewers to act -quickly and gives you time to address the issue fully. It also is an opportunity -to take a step back and reassess the test coverage. +While it can feel like a significant step backward to have your PR reverted, in most cases it is +much easier to reland the PR once a fix can be confirmed. Allowing a revert to land takes pressure +off of you and your reviewers to act quickly and gives you time to address the issue fully. It also +is an opportunity to take a step back and reassess the test coverage. ### Rollups +All reviewers are strongly encouraged to explicitly mark a PR as to whether or not it should be +part of a [rollup]. This is usually done either when approving a PR with `@bors r+ $ROLLUP_STATUS` +or with `@bors $ROLLUP_STATUS` where `$ROLLUP_STATUS` is substituted with one of the following: -All reviewers are strongly encouraged to explicitly mark a PR as to whether or -not it should be part of a [rollup]. This is usually done either when approving -a PR with `@bors r+ $ROLLUP_STATUS` or with `@bors $ROLLUP_STATUS` where -`$ROLLUP_STATUS` is substituted with one of the following: - -- `rollup=always`: These PRs are very unlikely to break tests or have - performance implications. Example scenarios: - - Changes are limited to documentation, comments, etc. that is highly - unlikely to fail a build. +- `rollup=always`: These PRs are very unlikely to break tests or have performance implications. + Example scenarios: + - Changes are limited to documentation, comments, etc. that is highly unlikely to fail a build. - Changes cannot have performance implications. - Your PR is not landing possibly-breaking or behavior altering changes. - - Feature stabilization without other changes is likely fine to rollup, - though. + - Feature stabilization without other changes is likely fine to rollup, though. - When in doubt do not use this option! -- `rollup=maybe`: This is the default if `@bors r+` does not specify any rollup - status at all. Use this if you have some doubt that the change won't break - tests. This can be used if you aren't sure if it should be one of the other - categories. Since this is the default, there is usually no need to explicitly - specify this, unless you are un-marking the rollup level from a previous - command. -- `rollup=iffy`: Use this for mildly risky PRs (more risky than "maybe"). - Example scenarios: - - The PR is large and non-additive (note: adding 2000 lines of completely - new tests is fine to rollup). - - Has platform-specific changes that are not checked by the normal PR - checks. +- `rollup=maybe`: This is the default if `@bors r+` does not specify any rollup status at all. Use + this if you have some doubt that the change won't break tests. This can be used if you aren't sure + if it should be one of the other categories. Since this is the default, there is usually no need + to explicitly specify this, unless you are un-marking the rollup level from a previous command. +- `rollup=iffy`: Use this for mildly risky PRs (more risky than "maybe"). Example scenarios: + - The PR is large and non-additive (note: adding 2000 lines of completely new tests is fine to + rollup). + - Has platform-specific changes that are not checked by the normal PR checks. - May be affected by MIR migrate mode. -- `rollup=never`: This should *never* be included in a rollup (**please** - include a comment explaining why you have chosen this). Example scenarios: +- `rollup=never`: This should *never* be included in a rollup (**please** include a comment + explaining why you have chosen this). Example scenarios: - May have performance implications. - - May cause unclear regressions (we would likely want to bisect to this PR - specifically, as it would be hard to identify as the cause from a rollup). + - May cause unclear regressions (we would likely want to bisect to this PR specifically, as it + would be hard to identify as the cause from a rollup). - Has a high chance of failure. - Is otherwise dangerous to rollup. - Messes too much with: @@ -545,12 +438,10 @@ a PR with `@bors r+ $ROLLUP_STATUS` or with `@bors $ROLLUP_STATUS` where - `rollup-`: this is equivalent to `rollup=maybe` ### Priority - -Reviewers are encouraged to set one of the rollup statuses listed above instead -of setting priority. Bors automatically sorts based on the rollup status (never -is the highest priority, always is the lowest), and also by PR age. If you do -change the priority, please use your best judgment to balance fairness and -urgency with other PRs. +Reviewers are encouraged to set one of the rollup statuses listed above instead of setting +priority. Bors automatically sorts based on the rollup status (never is the highest priority, +always is the lowest), and also by PR age. If you do change the priority, please use your best +judgment to balance fairness and urgency with other PRs. The following is some guidance for setting priorities: @@ -575,36 +466,31 @@ The following is some guidance for setting priorities: - Release promotions ### Expectations for r+ +bors privileges are binary: the bot doesn't know which code you are familiar with and what code +you are not. They must therefore be used with discretion. Do not r+ code that you do not know +well -- you can definitely **review** such code, but try to hand off reviewing to someone else +for the final r+. -bors privileges are binary: the bot doesn't know which code you are familiar -with and what code you are not. They must therefore be used with discretion. Do -not r+ code that you do not know well -- you can definitely **review** such -code, but try to hand off reviewing to someone else for the final r+. - -Similarly, never issue a `r=username` command unless that person has done the -review, and the code has not changed substantially since the review was done, -and that the person has explicitly indicated that another contributor can `r=` -on their behalf. +Similarly, never issue a `r=username` command unless that person has done the review, and the +code has not changed substantially since the review was done, and that the person has explicitly +indicated that another contributor can `r=` on their behalf. -Rebasing is fine and often necessary, but changes in functionality typically -require re-review. It is very helpful for the reviewer if the PR author can -produce a brief summary of what has changed since last review, in addition to -responding to individual review comments. +Rebasing is fine and often necessary, but changes in functionality typically require re-review. +It is very helpful for the reviewer if the PR author can produce a brief summary of what has +changed since last review, in addition to responding to individual review comments. Please refer to [bors documentation for bot usage](../infra/docs/bors.md). ## Social aspects of reviewing - -First and foremost, PR authors and compiler reviews alike are expected to uphold -the [Code of Conduct][coc]. Simply speaking, a reviewer is expected to be -respectful to the PR author, even if the reviewer disagrees with the changes. - -Reviewers are encouraged to consider matters from the perspectives of the PR -author too. If a change is stuck due to procedural reasons or reviewer bandwidth -for months without any resolution (including a resolution that the compiler -might not be ready to accept such a change at present time, but thank the PR -author for the contributions anyway), and accrues constant merge conflicts, it -can be very frustrating. +First and foremost, PR authors and compiler reviews alike are expected to uphold the [Code of +Conduct][coc]. Simply speaking, a reviewer is expected to be respectful to the PR author, even if +the reviewer disagrees with the changes. + +Reviewers are encouraged to consider matters from the perspectives of the PR author too. If a +change is stuck due to procedural reasons or reviewer bandwidth for months without any resolution +(including a resolution that the compiler might not be ready to accept such a change at present +time, but thank the PR author for the contributions anyway), and accrues constant merge conflicts, +it can be very frustrating. If some discussions are getting heated, ask the [moderation team](https://www.rust-lang.org/governance/teams/moderation) to step in. diff --git a/src/compiler/steering-meeting.md b/src/compiler/steering-meeting.md deleted file mode 100644 index 16073abeb..000000000 --- a/src/compiler/steering-meeting.md +++ /dev/null @@ -1,49 +0,0 @@ -# Compiler-team Steering Meeting - -## What is it? - -The "steering meeting" is a weekly meeting dedicated to planning and -high-level discussion. The meeting operates on a repeating schedule: - -- Week 1: Planning -- Week 2: Technical or non-technical discussion -- Week 3: Technical or non-technical discussion -- Week 4: Non-technical discussion - -The first meeting of the 4-week cycle is used for **planning**. The -primary purpose of this meeting is to **select the topics for the next -three meetings**. The topics are selected from a set of topic -proposals, which must be uploaded and available for perusal before the -meeting starts. The planning meeting is also an opportunity to check -on the "overall balance" of our priorities. - -The remaining meetings are used for design or general discussion. -Weeks 2 and 3 can be used for **technical** or **non-technical** -discussion; it is also possible to use both weeks to discuss the same -topic, if that topic is complex. **Week 4 is reserved for -non-technical topics**, so as to ensure that we are keeping an eye on -the overall health and functioning of the team. - -### Where do proposals come from? - -The team accepts proposals via an open submission process, -which is documented on [its own page](steering-meeting/submit.md) - -## Announcing the schedule - -After each planning meeting, the topics for the next three weeks are -added to the [compiler-team meeting calendar][c] and a blog post is -posted to the [Inside Rust blog][ir]. - -## When and where is it? - -See the [compiler team meeting calendar][c] for the canonical date and -time. The meetings take place in the #t-compiler stream on the -[rust-lang Zulip][z]. - -[c]: https://rust-lang.github.io/compiler-team/#meeting-calendar -[ir]: https://blog.rust-lang.org/inside-rust/ -[z]: https://rust-lang.zulipchat.com - -[rust-lang/rust#54818]: https://github.com/rust-lang/rust/issues/54818 -[compiler-team website]: https://rust-lang.github.io/compiler-team/about/triage-meeting/ diff --git a/src/compiler/steering-meeting/how-to-run-design.md b/src/compiler/steering-meeting/how-to-run-design.md deleted file mode 100644 index 416fb740e..000000000 --- a/src/compiler/steering-meeting/how-to-run-design.md +++ /dev/null @@ -1,38 +0,0 @@ -# How to run the design meeting - -## Week of the meeting - -* Announce the meeting in the triage meeting -* Skim over the list of proposals and ping people who have open - proposals to get their availability over the next few weeks -* Make sure that a write-up is available and nag the meeting person otherwise - -## Day of the meeting - -* Create a `design meeting YYYY.MM.DD` topic - * Ping `@t-compiler/meeting`, ideally 1h or so before the meeting actually starts, - to remind people - * Include a link to the design meeting write-up -* At the time of the meeting, return to the topic - * Ping `@t-compiler/meeting` to let people know the meeting is starting - * Include a link to the design meeting write-up -* We typically begin with a 5min announcement period - -To guide the meeting, create a shared hackmd document everyone can -view (or adapt an existing one, if there is a write-up). Use this to -help structure the meeting, document consensus, and take live -notes. Try to ensure that the meeting ends with sort of consensus -statement, even if that consensus is just "here are the problems, here -is a space of solutions and their pros/cons, but we don't have -consensus on which solution to take". - -## After the meeting - -* Post the final contents of the summary hackmd as minutes to the - [`minutes/design-meeting` directory in the compiler-team - repository][ct] -* (Optional) Create an Inside Rust blog post pointing people at the - minutes and maybe giving a few notes - -[ct]: https://github.com/rust-lang/compiler-team/tree/master/content/minutes/design-meeting - diff --git a/src/compiler/steering-meeting/how-to-run-planning.md b/src/compiler/steering-meeting/how-to-run-planning.md deleted file mode 100644 index b57e79834..000000000 --- a/src/compiler/steering-meeting/how-to-run-planning.md +++ /dev/null @@ -1,59 +0,0 @@ -# How to run the planning meeting - -## Week of the meeting - -* Announce the meeting in the triage meeting -* Skim over the list of proposals and ping people who have open - proposals to get their availability over the next few weeks - -## Day of the meeting - -* Create a `design meeting YYYY.MM.DD` topic - * Ping `@t-compiler/meeting`, ideally 1h or so before the meeting actually starts, - to remind people -* At the time of the meeting, return to the topic - * Ping `@t-compiler/meeting` to let people know the meeting is starting -* We typically begin with a 5min announcement period -* Visit the [compiler-team] repository to get a list of proposed meetings - -To actually make the final selection, we recommend - -* First, try to identify topics that are clear non-candidates - * for example, sometimes more investigative work (e.g., data gathering) is needed - * try to identify people to do those tasks - * other issues may be out of date, or clear non-starters, and they can be closed -* Next tackle technical design meetings, then non-technical - * Typical ratio is 2 technical, 1 non-technical, but this is not set in stone - * It's ok to have fewer than 3 meetings - -[compiler-team]: https://github.com/rust-lang/compiler-team/ - -## Announce the meetings - -For each scheduled meeting, create a calendar event: - -* invite key participants to the meeting -* set the location to `#t-compiler, Zulip` -* include a link to the design meeting issue in the event - -In the relevant issues, add the `meeting-scheduled` label and add a -message like: - -``` -In today's [planning meeting], we decided to schedule this meeting for **DATE**. - -[Calendar event] - -[planning meeting]: XXX link to Zulip topic -[Calendar event]: XXX link to calendar event -``` - -You can get the link to the calendar event by clicking on the event in -google calendar and selecting "publish". - -## Publish a blog post - -Add a blog post to the Inside Rust blog using the [template found on -the compiler-team repository][blog-template]. - -[blog-template]: https://github.com/rust-lang/compiler-team/blob/master/templates/planning-meeting-blog-post.md diff --git a/src/compiler/steering-meeting/submit.md b/src/compiler/steering-meeting/submit.md deleted file mode 100644 index ae1c4ae2b..000000000 --- a/src/compiler/steering-meeting/submit.md +++ /dev/null @@ -1,101 +0,0 @@ -# Submitting a proposal - -If you would like to submit a proposal to the steering meeting for -group discussion, read on! This page has all the details. - -## TL;DR - -In short, all you have to do is - -- [open an issue on the compiler-team repository][ct issues] - - use the template for meeting proposals - - you only need a few sentences to start, but by the time the meeting - takes place we typically expect a more detailed writeup, e.g. - using [this template][template] - -You don't have to have a lot of details to start: just a few sentences -is enough. But, especially for technical design discussions, we will -typically expect that some form of more detailed overview be made -available by the time the meeting takes place. - -## Examples of good candidates for discussing at the steering meeting - -Here are some examples of possible technical topics that would be -suitable for the steering meeting: - -- A working group has an idea to refactor the HIR to make some part of their - job easier. They have sketched out a proposal and would like feedback. -- Someone has encountered a problem that is really hard to solve with - the existing data structures. They would like feedback on a good - solution to their problem. -- Someone has done major refactoring work on a PR and they would like - to be able to explain the work they did and request review. - -Steering meetings are also a good place to discuss other kinds of proposals: - -- A proposal to move some part of the compiler into an out-of-tree crate. -- A proposal to start a new working group. - -Note that a steering meeting is **not** required to create a new -working group or an out-of-tree crate, but it can be useful if the -proposal is complex or controversial, and you would like a dedicated -time to talk out the plans in more detail. - -## Criteria for selection - -When deciding the topics for upcoming meetings, we must balance a number of things: - -- We don't want to spend time on design work unless there are known - people who will implement it and support it; this includes not only - the "main coder" but also a suitable reviewer. -- We don't want to take on "too many" tasks at once, even if there *are* people to - implement them. -- We also don't want to have active projects that will be "stepping on - each others' toes", changing the same set of code in deep ways. - -## Meetings are not mandatory - -It is perfectly acceptable to choose *not* to schedule a particular -slot. This could happen if (e.g.) there are no proposals available or -if nothing seems important enough to discuss at this moment. Note -that, to keep the "time expectations" under control, we should -generally stick to the same 4-week cycle and simply opt to skip -meetings, rather than (e.g.) planning things at the last minute. - -## Adding a proposal - -Proposals can be added by opening an issue on the [compiler-team -repository][ct issues]. There is an issue template for meeting -proposals that gives directions. The basic idea is that you open an -issue with a few sentences describing what you would like to talk -about. - -Some details that might be useful to include: - -* how complex of a topic you think this is -* people in the compiler team that you think should be present for the meeting - -### Expectations for the meeting - -By the time the meeting takes place, we generally would prefer to have -a more detailed write-up or proposal. You can find a [template] for -such a proposal here. This should be created in the form of a hackmd -document -- usually we will then update this document with the minutes -and consensus from the meeting. The final notes are then stored in the -[minutes] directory of the compiler-team repository. - -### Expectations for a non-technical proposal - -**The requirements for non-technical proposals are somewhat looser.** A -few sentences or paragraphs may well suffice, if it is sufficient to -understand the aims of the discussion. - -## Frequently asked questions - -**What happens if there are not enough proposals?** As noted above, -meetings are not mandatory. If there aren't enough proposals in some -particular iteration, then we can just opt to not discuss anything. - -[ct issues]: https://github.com/rust-lang/compiler-team/issues -[template]: https://github.com/rust-lang/compiler-team/blob/master/templates/steering-meeting-proposal.md -[minutes]: https://github.com/rust-lang/compiler-team/tree/master/content/minutes diff --git a/src/compiler/third-party-out-of-tree.md b/src/compiler/third-party-out-of-tree.md new file mode 100644 index 000000000..5cf1393d4 --- /dev/null +++ b/src/compiler/third-party-out-of-tree.md @@ -0,0 +1,156 @@ +# Third-party and Out-of-tree Crates Policy +This policy describes the guidelines for creating out-of-tree crates for use in the compiler and +using third-party crates within the compiler. + +## Out-of-tree crates +One of the primary goals of this policy is to ensure that there is consistency in how out-of-tree +crates used in the compiler are set up (at least, those maintained by the compiler team and living +in `rust-lang`) and that the experience is uniform across `rust-lang/rust` and these crates. + +### When should parts of the compiler be extracted into an out-of-tree crate? +This is left to the discretion of compiler team members but should be discussed with the rest of +the team, either through raising the question at the weekly triage meeting or asynchronously using +[an approval decision](./proposals-and-stabilizations.md). If the crate is a product of a working +group, there should already be agreement within the working group that an out-of-tree crate is +suitable. + +When considering creating an out-of-tree crate, it is worth balancing how general the crate should +be with the increased maintenance burden that this may bring if widely used. + +### Where should compiler crates live? +Out-of-tree compiler crates should be hosted in the `rust-lang` organization - this simplifies +integration with external infrastructure tooling and will inherit existing team permissions on +GitHub. It should be made clear in any documentation that the compiler team and any appropriate +working groups are responsible for the crate. It is not recommended to start with a prototype in +another organization or personal repository. + +### Can existing out-of-tree crates from personal accounts or other organizations be transferred? +Yes, this is encouraged. In order to do this, discuss this with the team and familiarize yourself +with [the GitHub documentation for repository transfers][repo_transfers] and then arrange to perform +the transfer. Once a transfer is complete, a redirect will exist in the original account or +organization and this will conflict with the names of any new forks of the transferred repository - +an email to GitHub is required to resolve this. + +[repo_transfers]: https://help.github.com/en/articles/transferring-a-repository + +### Who owns these crates? +It is desired that a compiler team member or working group has loose ownership over a crate so +that there are clear owners who are responsible for making sure that new versions are published and +that pull requests are reviewed. + +### What should these crates be named? +Crate naming will be discussed when new out-of-tree crates are proposed to the compiler team. + +Crate naming will differ on a case-by-case basis. Crates that are inherently tied to the +compiler would benefit from a name that is prefixed with `rustc_`. This is an indicator of how +stable the crate may be to prospective users. Other crates, which are more general-purpose, will +have names that are disentangled from the compiler. + +### Are there any limitations on the review policy for out-of-tree crates? +Generally, the working groups and team members that are primarily free to maintain the crate using +whatever practices are best suited to their group, however, there are some limitations so that there +is some uniformity across the compiler and out-of-tree crates: + +- Everyone with r+ on `rust-lang/rust` should be able to review and approve PRs. +- Where possible, only active participants in the crate (or related working group) need be on the + triagebot rotation for the crate. +- It is fine to have additional reviewers on the crate who do not otherwise have r+ for Rust as a + whole, if those reviewers are actively involved in the working group or crate maintenance. +- Major pull requests should have multiple reviewers. + +### What is required of an out-of-tree crate? +It is required that out-of-tree crates must: + +- Be dual-licensed with Apache 2.0 and MIT when maintained by the compiler team (as the compiler + is) unless there is a compelling reason to do otherwise. + - If another license is desired, this must be brought up when proposing the new crate for + compiler team members to agree. Prefer [licenses accepted by tidy][licenses], unless otherwise + required (ports of code from other projects, etc). +- Abide by Rust's code of conduct. +- Specify that the crate is maintained by the Rust compiler team and any appropriate working groups. + - In particular, this should detail the expected level of maintenance and stability for any + prospective users. + - This should also link to the working group details in this repository. +- Be added to the list at the bottom of this page. +- Follow semantic versioning. +- Use GitHub merge queues and `@triagebot`. +- Use labels that are compatible with the existing triage process. This will allow nominated issues + in your out-of-tree crate to be discussed during triage meetings. + - eg. `T-compiler`, `I-compiler-nominated` (a full list is to be decided) + +### Is there a requirement for community infrastructure for an out-of-tree crate? +There is no requirement that community infrastructure (such as Zulip servers/streams) be created for +out-of-tree crates. This may be desirable if an out-of-tree crate gains a large community of +contributors and users, but otherwise, the working group or compiler team streams should be used +initially. + +Linkifiers for auto-linking to issues and PRs on the primary Rust Zulip server can be added on +request. + +### Are there any recommendations for working with out-of-tree crates? +Recommendations for working with out-of-tree crates will be documented in the rustc-dev-guide (see +[rust-lang/rustc-dev-guide#285][guide_issue] for progress). + +### How should stabilization/semantic changes be handled in out-of-tree crates? +It is important to involve the language team in any changes in out-of-tree crates that would result +in stabilization or semantic changes to the language. Submodule changes in PRs to `rust-lang/rust` +should be labelled appropriately (eg. `relnotes`, `t-compiler`, `t-lang`) just as if the change +were implemented in `rust-lang/rust` directly, include a description of the changes when it is not +obvious to those unfamiliar with the compiler or the out-of-tree crate. + +[licenses]: https://github.com/rust-lang/rust/blob/master/src/tools/tidy/src/deps.rs#L10-L19 +[guide_issue]: https://github.com/rust-lang/rustc-dev-guide/issues/285 + +--- + +In summary, the process for establishing an out-of-tree crate is as follows: + +1. Where appropriate, discuss and confirm the need within the working group for the out-of-tree + crate. +1. Create a PR modifying this document to include the crate in the list below. Use + [`@rfcbot merge`](https://github.com/anp/rfcbot-rs#usage) to gain agreement from compiler + team members. +1. Create a new repository in the `rust-lang` organization using the teams repository. + 1. Ensure that the `access.teams.compiler` key is set to `'maintain'` so that the team have + appropriate permissions. + 1. Add a README describing the intended purpose of the crate, which team and working group are + responsible (link to their page in this repository) and the intended level of maintenance and + stability. + + > This crate is developed and maintained by the [Rust compiler team](https://github.com/rust-lang/compiler-team/tree/master/procedures) for use within + > `rustc`, in particular, it is the responsibility of the + > [`.template`](../../working-groups/template) working group. This crate [will have regular + > breaking changes and provides no stability guarantees|is intended to remain stable and have + > limited breaking changes]. + 1. Include the [LICENSE-APACHE][apache] and [LICENSE-MIT][mit] files from `rust-lang/rust`. + 1. Include or link the [CODE_OF_CONDUCT][coc] file from `rust-lang/rust`. + 1. Create a relevant `.gitignore` ([here's a sane default][gitignore]). + 1. Create `P-high`, `P-medium`, `P-low`, `I-compiler-nominated` and `T-compiler` labels. +1. Perform any initial development required before integration with rustc. +1. Publish initial version, following semantic versioning. +1. Add the crate as a dependency to the appropriate in-tree crate and start using. + +[gitignore]: https://gitignore.io/api/vim,rust,emacs,clion,visualstudio,visualstudiocode +[triagebot]: https://github.com/rust-lang/rust/blob/master/triagebot.toml +[apache]: https://github.com/rust-lang/rust/blob/master/LICENSE-APACHE +[coc]: ../../about/code_of_conduct +[mit]: https://github.com/rust-lang/rust/blob/master/LICENSE-MIT + +## Third-party crates +It is sometimes desirable to use the functionality of an existing third-party crate in the compiler. + +### When can a third-party crate be added as a compiler dependency? +It is desirable that a third-party crate being included in the compiler is well-maintained and that, +where possible, a compiler team member is added as a maintainer. You should consult with the +rest of the compiler team before making this decision. + +### What about third-party dependencies to out-of-tree crates? +The same policies apply to all compiler-team-maintained crates used in the compiler. + +## List of out-of-tree crates +This section contains the list of existing out-of-tree, compiler team-maintained crates: + + - [`rust-lang/chalk`](https://github.com/rust-lang/chalk/) + - [`rust-lang/polonius`](https://github.com/rust-lang/polonius/) + - [`rust-lang/measureme`](https://github.com/rust-lang/measureme/) + - [`rust-lang/thorin`](https://github.com/rust-lang/thorin/) diff --git a/src/compiler/triage-meeting.md b/src/compiler/triage-meeting.md deleted file mode 100644 index f9452932d..000000000 --- a/src/compiler/triage-meeting.md +++ /dev/null @@ -1,27 +0,0 @@ -# Compiler-team Triage Meeting - -## What is it? - -The triage meeting is a weekly meeting where we go over the open -issues, look at regressions, consider beta backports, and other such -business. In the tail end of the meeting, we also do brief check-ins -with active working groups to get an idea what they've been working -on. - -## When and where is it? - -See the [compiler team meeting calendar][c] for the canonical date and -time. The meetings take place in the #t-compiler stream on the -[rust-lang Zulip][z]. - -[c]: https://rust-lang.github.io/compiler-team/#meeting-calendar -[z]: https://rust-lang.github.io/compiler-team/about/chat-platform/ - -## Where can I lean more? - -The meeting procedure is documented in [rust-lang/rust#54818]. - -The working group check-in schedule is available on the [compiler-team website]. - -[rust-lang/rust#54818]: https://github.com/rust-lang/rust/issues/54818 -[compiler-team website]: https://rust-lang.github.io/compiler-team/about/triage-meeting/