-
Notifications
You must be signed in to change notification settings - Fork 327
Add the new *Clippy-Contributors* team 🎉 #1411
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
Conversation
Special thanks to our new members, who have already helped so much in Clippy: * @GuillaumeGomez * @J-ZhengLi * @m-rph * @pitaj
teams/clippy-contributors.toml
Outdated
| repo = "https://github.com/rust-lang/rust-clippy" | ||
|
|
||
| [[zulip-groups]] | ||
| name = "T-clippy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
| name = "T-clippy" | |
| name = "T-clippy-contributors" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to include them in the normal T-clippy Zulip group. We get very few pings, like one every two weeks when we have a team meeting, where the contributors are also invited to.
Even if another team would ping us for a design decision, the feedback from them would be useful. That's why I set it to T-clippy. Though, I'm open to change this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you need to add clippy-contributors to the Zulip group definition in clippy.toml using the extra-teams key. Right now, adding a duplicate zulip group definition causes the last definition to be processed to override all other definitions which is obviously a bug. I would argue we need to throw a validation error in this case instead of allowing the group definitions be split across files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, I would go with Ryan's original suggestion and just add them to T-clippy-contributors. No need to ping them for things they don't necessarily have to pay attention to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And another bug was found thanks to Clippy¹
¹kind of
flip1995
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, apart from Ryan's comment. Should be merged together with/after the RFC in the Clippy repo.
|
I'll wait until the RFC is officially merged before merging this. Ping me if I forget! |
|
Hey @rylev, we just merged the RFC. Can you merge this as well to make the team official? :D |
This PR adds the new Clippy-Contributors team which has been proposed in rust-lang/rust-clippy#12481 and was accepted on Zulip yesterday! 🎉
As part of this PR, I also want to welcome the four initial heroes, who will start be part of this team:
Thank you, for all the work you already put into Clippy. We're happy to have you! ❤️
cc: rust-lang/clippy
r? @flip1995 & @Manishearth