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

ci: ensure all tool maintainers are assignable on issues #64119

Merged
merged 5 commits into from
Sep 16, 2019

Conversation

pietroalbini
Copy link
Member

GitHub only allows people explicitly listed as collaborators on the repository or who commented on the issue/PR to be assignees, failing to create the issue if non-assignable people are assigned.

This adds an extra check on CI to make sure all the people listed as tool maintainers can be assigned to toolstate issues. The check won't be executed on PR builds due to the lack of a valid token.

r? @kennytm

@pietroalbini
Copy link
Member Author

pietroalbini commented Sep 3, 2019

We can't merge this yet as CI will fail due to these errors:

rust-by-example: @marioidival is not assignable
rust-by-example: @projektir is not assignable
embedded-book: @adamgreig is not assignable
embedded-book: @andre-richter is not assignable
embedded-book: @jamesmunns is not assignable
embedded-book: @korken89 is not assignable
embedded-book: @ryankurte is not assignable
embedded-book: @thejpster is not assignable
embedded-book: @therealprof is not assignable
rustc-guide: @mark-i-m is not assignable
rustc-guide: @amanjeev is not assignable
rustfmt: @topecongiro is not assignable

@kennytm kennytm added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 3, 2019
@pietroalbini
Copy link
Member Author

For everyone pinged by this comment: toolstate is configured to open a new issue when a tool breaks, assigning the people who usually fix those breakages. Unfortunately GitHub only allows assigning people with explicit access to a repo, so we need to grant everyone who's going to be assigned access to the repo.

We're going to grant "read" access to the repo, which doesn't add new real permissions (well, it's a public repository...) while allowing y'all to be assigned. Since adding permissions to individuals is not a good practice we should get GitHub teams in the rust-lang org for each tool and then grant access to those teams.


Opened PRs in the team repo to synchronize wg-rustfmt and rust-by-example with GitHub.

@nikomatsakis which teams should @mark-i-m and @amanjeev be part of for rustc-guide?

@therealprof could you put the people responsible for the embedded book in a separate team/wg on the team repo? Then we can automatically synchronize that team with the rust-lang GitHub organization.

@therealprof
Copy link
Contributor

@pietroalbini The @rust-embedded/resources team is responsible for managing the book, is that granular enough for this purpose?

@pietroalbini
Copy link
Member Author

@therealprof yeah, but it needs to be on the rust-lang org. If you setup the team on the rust-lang/team repo we can synchronize it with GitHub.

src/tools/publish_toolstate.py Outdated Show resolved Hide resolved
src/tools/publish_toolstate.py Outdated Show resolved Hide resolved
src/tools/publish_toolstate.py Outdated Show resolved Hide resolved
src/tools/publish_toolstate.py Show resolved Hide resolved
src/tools/publish_toolstate.py Outdated Show resolved Hide resolved
therealprof added a commit to therealprof/team that referenced this pull request Sep 4, 2019
Hopefully this satisfies the requirements of
rust-lang/rust#64119

Signed-off-by: Daniel Egger <[email protected]>
@therealprof
Copy link
Contributor

@pietroalbini Like rust-lang/team#107 ?

therealprof added a commit to therealprof/team that referenced this pull request Sep 4, 2019
Hopefully this satisfies the requirements of
rust-lang/rust#64119

Signed-off-by: Daniel Egger <[email protected]>
@pietroalbini
Copy link
Member Author

Yep! rustfmt and embedded-book should now be fixed.

@pietroalbini
Copy link
Member Author

@kennytm addressed review comments.

@kennytm
Copy link
Member

kennytm commented Sep 4, 2019

r=me after everyone is confirmed assignable.

@pietroalbini
Copy link
Member Author

Also removed @projektir as they don't contribute to rust by example anymore. People who still need to get permissions:

GitHub only allows people explicitly listed as collaborators on the
repository or who commented on the issue/PR to be assignees, failing to
create the issue if non-assignable people are assigned.

This adds an extra check on CI to make sure all the people listed as
tool maintainers can be assigned to toolstate issues. The check won't be
executed on PR builds due to the lack of a valid token.
They don't contribute to rust-by-example anymore.
@pietroalbini
Copy link
Member Author

All the permissions are properly configured 🎉

@bors r=kennytm

@bors
Copy link
Contributor

bors commented Sep 16, 2019

📌 Commit feab38070777e55b4f4fa76c383aa3274b1e1200 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 16, 2019
@pietroalbini
Copy link
Member Author

@bors r=kennytm

@bors
Copy link
Contributor

bors commented Sep 16, 2019

📌 Commit ce451b9 has been approved by kennytm

Centril added a commit to Centril/rust that referenced this pull request Sep 16, 2019
…ntainers, r=kennytm

ci: ensure all tool maintainers are assignable on issues

GitHub only allows people explicitly listed as collaborators on the repository or who commented on the issue/PR to be assignees, failing to create the issue if non-assignable people are assigned.

This adds an extra check on CI to make sure all the people listed as tool maintainers can be assigned to toolstate issues. The check won't be executed on PR builds due to the lack of a valid token.

r? @kennytm
bors added a commit that referenced this pull request Sep 16, 2019
Rollup of 10 pull requests

Successful merges:

 - #63955 (Make sure interned constants are immutable)
 - #64028 (Stabilize `Vec::new` and `String::new` as `const fn`s)
 - #64119 (ci: ensure all tool maintainers are assignable on issues)
 - #64444 (fix building libstd without backtrace feature)
 - #64446 (Fix build script sanitizer check.)
 - #64451 (when Miri tests are not passing, do not add Miri component)
 - #64467 (Hide diagnostics emitted during --cfg parsing)
 - #64497 (Don't print the "total" `-Ztime-passes` output if `--prints=...` is also given)
 - #64499 (Use `Symbol` in two more functions.)
 - #64504 (use println!() instead of println!(""))

Failed merges:

r? @ghost
@bors bors merged commit ce451b9 into rust-lang:master Sep 16, 2019
@pietroalbini pietroalbini deleted the validate-toolstate-maintainers branch September 16, 2019 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants