-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Issue #8823 - remove bots from contributors list #8828
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
scripts/release.py
Outdated
@@ -22,6 +22,11 @@ def announce(version): | |||
|
|||
contributors = set(stdout.splitlines()) | |||
|
|||
# remove strings within contributors that have substring "[bot]" | |||
for name in 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.
for name in contributors: | |
for name in list(contributors): |
Otherwise we will change the size of the set during iteration and raise an error.
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.
But probably it is easier to update line 23 to:
contributors = set(x for x in stdout.splitlines() if "[bot]" not in x)
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.
Yeah I agree. I think the one-liner works well and looks nicer. Should I make a new commit?
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.
We typically squash PRs into a single commit when merging, so feel free to just push a second commit to the same branch (which will automatically update the PR).
As an aside, I'd probably use name
instead of x
, and if not name.endswith("[bot]")
as I think it's always used as a suffix.
for more information, see https://pre-commit.ci
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.
Looks great now, thanks! 🎉 One step closer to the 7.0 release!
No description provided.