Skip to content

Commit a2e6ba0

Browse files
michaelwoeristerjieyouxu
authored andcommitted
Apply review suggestions
1 parent 430ec50 commit a2e6ba0

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

src/compiler/reviews.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ This section lists a few common cases together with guidance on how to deal with
8787
- If you don't know the code well or already have too much on your plate,
8888
ask rustbot to roll the dice again via `r? compiler`.
8989

90-
You can also always ask for help on the `#t-compiler/reviews` Zulip stream for finding a reviewer.
90+
You can also always ask for help on the `#t-compiler` Zulip stream for finding a reviewer.
9191
That being said, you are always welcome to do an initial review (to the extent you are comformtable with)
9292
and then pass the PR on to the final reviewer.
9393
This way the PR author will get helpful feedback sooner and subsequent reviewers will have less work to do.
@@ -99,7 +99,7 @@ This section lists a few common cases together with guidance on how to deal with
9999
You can also nominate the PR for discussion in the compiler team's triage meeting by tagging it `I-nominated`.
100100
If you nominate a PR please make sure to state a concrete question for the compiler team to discuss.
101101

102-
* ### Intransparent discussion and rationale
102+
* ### Discussion or rationale is too intransparent
103103
Sometimes there are PRs that seem to be the result of some prior discussion, with no description or rationale.
104104
They usually have a title like "Change X" and the only content of the PR message is "r? @xyz".
105105
Even though the change might make sense and may even have been suggested by a compiler team member this is not good form.
@@ -141,14 +141,15 @@ This section lists a few common cases together with guidance on how to deal with
141141
* ### Nobody understands the code that's being changed
142142
Sometimes there is a bug in some code that nobody understands anymore.
143143
The original authors are unavailable and it is hard to gauge the implications of a proposed fix.
144-
In such a case it is a good idea for reviewers to ask on the `#t-compiler/reviews` Zulip stream for additional help.
144+
In such a case it is a good idea for reviewers to I-nominate the PR (if they intend to stay the
145+
main reviewer) or assign a compiler team lead to the issue and add the S-waiting-on-team label.
146+
In both cases, the PR will be brought in the weekly triage meeting.
145147
It is also especially valuable to gather and document as much information as possible about the issue,
146148
such as a description of the problem being fixed, points of unclarity, potential risks,
147149
alternatives that have been considered, et cetera.
148150
Reviewers should ask PR authors to add this kind of information as comments in the code
149151
and/or to the PR message (which will become part of the git commit history).
150152

151-
152153
[mcp]: https://forge.rust-lang.org/compiler/mcp.html
153154
[whats-a-major-change]: https://forge.rust-lang.org/compiler/mcp.html#what-constitutes-a-major-change
154155

0 commit comments

Comments
 (0)