-
Notifications
You must be signed in to change notification settings - Fork 429
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
CONTRIB: Automate AUTHORS file update #10308
base: master
Are you sure you want to change the base?
Conversation
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.
Can we add this script to CI, so that if new author is adding a commit and it is not in AUTHORS file, the CI fails?
adding to CI imposes PR creators to have their commits under exact email, or name, could this become a problem at some point? |
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.
Nice
contrib/authors_update.sh
Outdated
if [ "$(git log --no-merges --oneline "$range" | wc -l)" -eq 0 ] | ||
then | ||
echo "Error: empty range \"$range\"" | ||
exit 1 | ||
fi | ||
|
||
# Failure triggered if range is not valid | ||
git --no-pager log --no-merges --oneline --pretty=format:"%h %an <%ae> %s" \ | ||
"$range" |
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.
can we do the range validity check in one git command? and if only for validity, why need pretty format?
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.
ok make sense, retested empty/bad range and patched, that removed the commit list, I think that's also ok since it can be very long.
git log --no-merges --pretty=format:"%an%x09%ae" "$range" | sort | uniq | \ | ||
while IFS=$'\t' read -r name email; do | ||
line="$name <$email>" | ||
if ! grep -iqw "$email" "$tmp" && ! grep -iq "$name" "$tmp"; then |
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.
can you pls add comment what this does?
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.
added
contrib/authors_update.sh
Outdated
|
||
if [ ! -w AUTHORS ] | ||
then | ||
echo "AUTHORS file not accessible" |
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.
is not writable
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.
fixed, but -w also covers the case where file does not exist
contrib/authors_update.sh
Outdated
|
||
set -eEu -o pipefail | ||
|
||
range="${1?Provide commit range like orig/v1.17..orig/v1.18}" |
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.
like
-> , for example:
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.
fixed
contrib/authors_update.sh
Outdated
lines=$(git log --no-merges --pretty=format:"%an%x09%ae" "$range" | sort | uniq) | ||
if [ -z "$lines" ] | ||
then | ||
echo "Error: empty range \"$range\"" |
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.
provided range is empty
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.
fixed
What?
Automate AUTHORS updates according to documentation.
Why?
Seems better to try to have common approach.
How?