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

Policy: Require MCP and relevant team nominations for adding (ecosystem, custom codegen backend) testing jobs that would block PR/Merge CI and require documenting failure protocol #845

Open
1 of 3 tasks
jieyouxu opened this issue Mar 3, 2025 · 1 comment
Labels
major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Mar 3, 2025

Proposal

Terminology

  • "Ecosystem test jobs": test jobs that aren't specifically exercising test suites against supported targets as listed in Platform Support docs. Examples at time of writing: the Fuchsia and Rust for Linux test jobs.
  • "Custom codegen backend test jobs": test jobs that are primarily intended to exercise test suites of (out-of-tree) codegen backends. Examples at time of writing: the cranelift and gcc codegen backends.

Policy: require compiler sign-off and reviews from other relevant teams for adding blocking ecosystem, custom codegen backend) test jobs to rust-lang/rust

Require a compiler MCP for adding new (ecosystem or custom codegen backend) test jobs (see e.g. https://rustc-dev-guide.rust-lang.org/tests/ecosystem.html) to https://github.com/rust-lang/rust that are blocking. If a job may fail due to non-compiler but rust-lang/rust-related causes (e.g. rustdoc, libs or bootstrap), please also nominate (via I-{team}-nominated) the proposal to add a new testing job for other teams by filing an issue about adding the blocking test job on https://github.com/rust-lang/rust. The test job may be added if the MCP is accepted and if there are no outstanding concerns from the nominated teams.

This policy is because if a test job is blocking, it affects everyone, not just the contributors who proposed to add the test job for $reasons.

When opening an MCP for the purpose of adding a new (ecosystem, custom codegen backend) test job, it is required that the author:

  • Establish a ping group (and configured on rust-lang/rust) who is expected to consult on the test job failures.
  • Open a corresponding issue on https://github.com/rust-lang/rust and nominate it for relevant teams via I-{team}-nominated labels. For instance, if the new test job may break due to rustdoc changes, then it should be nominated to include rustdoc team review as well.
  • Include justification for the blocking test job in the MCP:
    • The reason(s) for adding the test job.
    • The reason(s) for the test job being blocking (for PR CI and/or full CI) must be explained.
    • The expected failure protocol.
      • How are failures expected to be handled? By test job maintainers, or other contributors (e.g. PR authors)?
      • Explicitly document how to request advice from the ping group.
      • How are unintentional/intentional breakages expected to be handled?
      • Can the job be temporarily disabled for PR CI or Full CI?
      • Are other (compiler) contributors expected to fix the failures?
      • How should intentional/unintentional test job breakages be handled?
  • The author is further expected to submit documentation for the test job to rustc-dev-guide (either to the https://github.com/rust-lang/rustc-dev-guide/ repo itself, or alongside the rust-lang/rust PR adding the test job).
    • Basically, the answers to questions required by the MCP above.
    • TODO: add a custom test job template to rust-lang/rustc-dev-guide to make it less likely to forget important bits.

This policy is retroactive for existing (ecosystem, custom codegen backend) testing jobs

This requirement is intended to be retroactive. We will file MCPs for the existing (ecosystem) test jobs (at the time of writing, the Fuchsia test job and the Rust for Linux test job). If they are accepted, then no actions need to be done. If there are outstanding blocking concerns, then we may remove the existing (ecosystem) test jobs if the concerns cannot be resolved in a timely fashion.

Mentors or Reviewers

@davidtwco and @wesleywiser

This should also be discussed with T-{rustdoc,infra,bootstrap,libs} reviewers.

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

@jieyouxu jieyouxu added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels Mar 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2025

Important

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

@rfcbot concern reason-for-concern 
<description of the concern> 

Concerns can be lifted with:

@rfcbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Mar 3, 2025
@jieyouxu jieyouxu changed the title Policy: Require MCP for adding (ecosystem) testing jobs that would block PR/Merge CI and require documenting failure protocol Policy: Require MCP for adding (ecosystem, custom codegen backend) testing jobs that would block PR/Merge CI and require documenting failure protocol Mar 3, 2025
@jieyouxu jieyouxu changed the title Policy: Require MCP for adding (ecosystem, custom codegen backend) testing jobs that would block PR/Merge CI and require documenting failure protocol Policy: Require MCP and relevant team nominations for adding (ecosystem, custom codegen backend) testing jobs that would block PR/Merge CI and require documenting failure protocol Mar 3, 2025
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

No branches or pull requests

3 participants