Sync organization members #1868
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pietroalbini
requested review from
Mark-Simulacrum,
jdno and
marcoieni
as code owners
Dry-run check results |
|
It's removing rust-lang-owner and other bots |
marcoieni
force-pushed
the
sync-org-members
branch
from
1bbac75 to
b8049db
Compare
|
I tried pushing one commit that also updated master, but failed miserably, so I reverted it 😅 |
marcoieni
force-pushed
the
sync-org-members
branch
from
b8049db to
3e935a6
Compare
|
Now Kobzol is not in the authors of the commit anymore. Sorry about that but that's the maximum I can get with my git skills 😂 |
marcoieni
force-pushed
the
sync-org-members
branch
2 times, most recently
from
ebdec80 to
e05929e
Compare
marcoieni
force-pushed
the
sync-org-members
branch
2 times, most recently
from
b8a431b to
c9d7993
Compare
tshepang
reviewed
Jul 15, 2025
marcoieni
force-pushed
the
sync-org-members
branch
2 times, most recently
from
e76e204 to
6a05230
Compare
Kobzol
reviewed
Jul 15, 2025
config.toml
Outdated
|
|
||
| # GitHub accounts that are allowed to stay in the orgs, | ||
| # even if they may not be members of any team. | ||
| allowed-org-members = [ |
There was a problem hiding this comment.
nit: allowed makes it sound like only these org members may be a part of an org :D Which of course doesn't make sense, and the comment does clarify it. But I don't have a great alternative. Maybe just special-org-members? But it doesn't really matter.
There was a problem hiding this comment.
Special also doesn't clarify much, maybe call it dont-remove-from-orgs = []?
config.toml
Outdated
| "rustbot", | ||
|
|
||
| # Infra admins. | ||
| "jdno", |
There was a problem hiding this comment.
Hmm, we start having a bunch of places where we hardcode infra-admins. Maybe it would be better if we loaded the actual contents of the infra-admins team (so that we hardcode only the team in code, but not its members) in code and ignored them?
There was a problem hiding this comment.
This is a good point, but also, I'm not sure if hardcoding the infra-admins team in the code is the best either. As we are not managing org owners with the team repo for now, I'd skip removing any org owner, and do a manual pass removing inactive org owners.
There was a problem hiding this comment.
We already hardcode infra-admins for CODEOWNERS, but ignoring org owners is a good point.
rust_team_data/src/v1.rs
Outdated
| use indexmap::IndexMap; | ||
| use serde::{Deserialize, Serialize}; | ||
|
|
||
| pub static BASE_URL: &str = "https://team-api.infra.rust-lang.org/v1"; | ||
|
|
||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)] |
There was a problem hiding this comment.
I don't think we need to expose this to the JSON API? It's an internal implementation detail. Can't you just pass &Config from team to sync-team in run_sync_team?
marcoieni
force-pushed
the
sync-org-members
branch
4 times, most recently
from
8e30e03 to
651601f
Compare
|
addressed the feedback, thanks. Ready for another look 👍 |
fmease
reviewed
Jul 17, 2025
marcoieni
force-pushed
the
sync-org-members
branch
2 times, most recently
from
8fd1430 to
2492b40
Compare
pietroalbini
commented
Jul 17, 2025
marcoieni
force-pushed
the
sync-org-members
branch
from
2492b40 to
91501b9
Compare
|
Triage: waiting on the communication part #t-infra > removing inactive members from the org. |
jieyouxu
added
needs-infra-admin-review
if you want more than one author to be listed, you can use |
marcoieni
force-pushed
the
sync-org-members
branch
from
91501b9 to
78a61b8
Compare
marcoieni
approved these changes
Aug 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Re-submitting #1866 again as requested:
cc @Kobzol
Discussed in #t-infra > removing inactive members from the org.