-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Prevent commitcheck.sh from running twice #5952
Conversation
@dinatale2, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @FransUrbo and @ryao to be potential reviewers. |
8fdfb26
to
3be4650
Compare
Just updated the regexes to improve matching tagged lines. |
scripts/commitcheck.sh
Outdated
@@ -16,7 +16,7 @@ function test_url() | |||
# check for a tagged line | |||
function check_tagged_line() | |||
{ | |||
regex='^\s*'"$1"':\s+\S+\s+\<\S+\>' | |||
regex='^\s*'"$1"':\s[-A-Za-z0-9 ]+\s<\S+>$' |
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.
This regex doesn't look broad enough. I'd suggest running it over the email addresses from the last couple hundred commits and makes sure it works. For example, it needs to contain .
.
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.
Currently, the email portion of the regex might be too broad. It currently doesn't enforce that @
is present, it just allows 1 or more non-whitespace characters to exist between the < >
. Do you want me to enforce that what shows up there is some form of email?
I'm also broadening the name portion of the regex.
A stray semicolon was causing commitcheck.sh to run twice when running make checkstyle. Updated regexes for matching tagged lines. Signed-off-by: Giuseppe Di Natale <[email protected]>
3be4650
to
09f57af
Compare
Description
commitcheck.sh
runs twice when running amake checkstyle
. A stray semicolon in theshellcheck
make target is causing the problem.How Has This Been Tested?
On a CentOS 7 VM.
Types of changes
Checklist:
Signed-off-by
.