Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add policy detailing review procedure for target-specific code #61

Merged
merged 1 commit into from
Jan 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- [When to `#[inline]`](./policy/inline.md)
- [Doc alias policy](./policy/doc-alias.md)
- [Safety comments policy](./policy/safety-comments.md)
- [Reviewing target-specific code](./policy/target-code.md)

- [Tricky situations]()
- [Drop and `#[may_dangle]`](./tricky/may-dangle.md)
Expand Down
25 changes: 25 additions & 0 deletions src/policy/target-code.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Reviewing target-specific code

When reviewing target-specific code, depending on the [tier] of the target in
question, different level of scrutiny is expected from reviewers.

For tier 1 targets, the reviewer should perform a full review of the code.
Essentially treat the code as *not* platform specific.

For tier 2 and tier 3 targets, the reviewer should confirm that the code:
Copy link
Member

Choose a reason for hiding this comment

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

Could say "at least" here.

Comment on lines +6 to +9
Copy link
Member

@dtolnay dtolnay Jan 20, 2024

Choose a reason for hiding this comment

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

Briefly reiterating the platform support policy would help motivate this guidance. Something like:

For tier 1 targets, the reviewer should perform a full review of the code. Essentially treat the code as not platform specific. Tier 1 code is considered "guaranteed to work", and code review is an important aspect of how we uphold this guarantee.

Tier 2 targets are considered "guaranteed to build". When reviewing tier 2 and tier 3 target-specific code, we don't ask thorough scrutiny but the reviewer should confirm that the code:


* Only affects 1 or more of such targets (i.e., is truly target-specific)
* Does not introduce new licensing hazards (e.g., license headers or similar)
* Is either proposed by a target maintainer[^1] or has pinged and received +1s
from at least one target maintainer. Where no maintainer is present, look for
whether the author is reputable and/or affiliated with the target in some way
(e.g., authored original code, works for a company maintaining the target, etc.).

Note that this review does *not* include checking for correctness or for code
quality. We lack the review bandwidth or expertise to perform detailed reviews
of tier 2 and tier 3 targets.

[^1]: Target maintainers are listed for most targets in the [platform support] documentation.

[tier]: https://doc.rust-lang.org/nightly/rustc/platform-support.html
[platform support]: https://doc.rust-lang.org/nightly/rustc/platform-support.html