ci(check-whitespace): update stale file top comments
PATCH 1/1 ci(check-whitespace): update stale file top comments
From: hakre <[email protected]>
Part of these two recent commits
1. a066a90db6 (ci(check-whitespace): restrict to the intended commits, 2021-07-15)
2. cc00362125 (ci(check-whitespace): stop requiring a read/write token, 2021-07-15)
are well written messages that reflect the changes (compare: [1]).
Unfortunately those commits left the description in top file comments unchanged which are still showing the previous picture.
To better display the current workflow upfront, those comments now reflect that:
1. full (not shallow) clone to steadily check the intended commits
2. communicated result is the exit status (not a comment in the PR)
[1]: https://git-scm.com/docs/SubmittingPatches#describe-changes
---
ci(check-whitespace): update stale file top comments
Part of these two recent commits
1. a066a90db6 (ci(check-whitespace): restrict to the intended commits, 2021-07-15)
2. cc00362125 (ci(check-whitespace): stop requiring a read/write token, 2021-07-15)
are well written messages that reflect the changes (compare: 1
[https://git-scm.com/docs/SubmittingPatches#describe-changes]).
Unfortunately those commits left the description in top file comments unchanged which are still showing the previous picture.
To better display the current workflow upfront, those comments now reflect that:
1. full (not shallow) clone to steadily check the intended commits
2. communicated result is the exit status (not a comment in the PR)
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1138%2Fhakre%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1138/hakre/patch-1-v1
Pull-Request: https://github.com/git/git/pull/1138
.github/workflows/check-whitespace.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
.github/workflows/check-whitespace.yml +2 -2
@@ -1,8 +1,8 @@ name: check-whitespace -# Get the repo with the commits(+1) in the series. +# Get the repo with all commits to steady catch the series. # Process `git log --check` output to extract just the check errors. -# Add a comment to the pull request with the check errors. +# Give status 2 on check errors. on: pull_request:
base-commit: 5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8
Signature · 1 line
gitgitgadget
Hi Hans,
Quoted 1 line · hakre via GitGitGadget
From: hakre <[email protected]>
As per https://git-scm.com/docs/SubmittingPatches#sign-off:
Please don’t hide your real name.
I strongly suspect your real name to be Hans Krentel, not hakre.
Quoted 37 lines
Part of these two recent commits
1.
a066a90db6(ci(check-whitespace): restrict to the intended commits, 2021-07-15)
2.cc00362125(ci(check-whitespace): stop requiring a read/write token, 2021-07-15)are well written messages that reflect the changes (compare: [1]).
Unfortunately those commits left the description in top file comments unchanged which are still showing the previous picture.
To better display the current workflow upfront, those comments now reflect that:
1. full (not shallow) clone to steadily check the intended commits
2. communicated result is the exit status (not a comment in the PR)[1]: https://git-scm.com/docs/SubmittingPatches#describe-changes
CC Johannes Schindelin <[email protected]>Signed-off-by hakre <[email protected]>---
ci(check-whitespace): update stale file top commentsPart of these two recent commits
1.
a066a90db6(ci(check-whitespace): restrict to the intended commits, 2021-07-15)
2.cc00362125(ci(check-whitespace): stop requiring a read/write token, 2021-07-15)are well written messages that reflect the changes (compare: 1
[https://git-scm.com/docs/SubmittingPatches#describe-changes]).Unfortunately those commits left the description in top file comments unchanged which are still showing the previous picture.
Quoted 7 lines
To better display the current workflow upfront, those comments now reflect that:
1. full (not shallow) clone to steadily check the intended commits
2. communicated result is the exit status (not a comment in the PR)Signed-off-by hakre [email protected]
If you send a new iteration, please replace the first comment on your PR by a cover letter. You can also delete the comment's contents instead.
Quoted 12 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1138%2Fhakre%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1138/hakre/patch-1-v1
Pull-Request: https://github.com/git/git/pull/1138.github/workflows/check-whitespace.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-).github/workflows/check-whitespace.yml +0
-0
.github/workflows/check-whitespace.yml line 3
Quoted 4 lines
.github/workflows/check-whitespace.yml +1
-1@@ -1,8 +1,8 @@ name: check-whitespace -# Get the repo with the commits(+1) in the series. +# Get the repo with all commits to steady catch the series.
I am not a native English speaker, but "to steady catch" strikes me as not quite English. I would suggest something like this instead:
Get the repository with all commits to ensure that we can analyze all of the commits contributed via the Pull Request.
.github/workflows/check-whitespace.yml line 5
Quoted 3 lines
.github/workflows/check-whitespace.yml +1
-1# Process `git log --check` output to extract just the check errors. -# Add a comment to the pull request with the check errors. +# Give status 2 on check errors.
Is it really interesting that the exit code 2 is used? Or is it more interesting that the job will exit with failure if the check produces errors? I think it would sound better as:
Exit with failure upon white-space issues.
Ciao,
Johannes
Quoted 8 lines
.github/workflows/check-whitespace.yml +0
-0on: pull_request:base-commit:
5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8--
gitgitgadget
Quoted 11 lines · Johannes Schindelin
Hi Hans,
Quoted 1 line · hakre via GitGitGadget
From: hakre <[email protected]>
As per https://git-scm.com/docs/SubmittingPatches#sign-off:
Please don’t hide your real name.
I strongly suspect your real name to be Hans Krentel, not hakre.
Thanks for pointing it out. I have a feeling that we see more submissions with this problem via GGG than other avenues, and I do not want to be the only one who is enforcing that rule. Very much appreciated.
Quoted 8 lines
Quoted 8 lines
Part of these two recent commits
1.
a066a90db6(ci(check-whitespace): restrict to the intended commits, 2021-07-15)
2.cc00362125(ci(check-whitespace): stop requiring a read/write token, 2021-07-15)are well written messages that reflect the changes (compare: [1]).
The above may not be incorrect per-se, but other than the fact that what this patch fixes came from these two commits, I find it largely irrelevant.
Quoted 2 lines
Quoted 2 lines
Unfortunately those commits left the description in top file comments unchanged which are still showing the previous picture.
Earlier a066a90d (ci(check-whitespace): restrict to the intended commits, 2021-07-14) changed the check-whitespace task to stop using a shallow clone, and cc003621 (ci(check-whitespace): stop requiring a read/write token, 2021-07-14) changed the way how the errors the task discovered is signaled back to the user.
They however forgot to update the comment that outlines what is done in the task. Correct them.
would perhaps be sufficient?
Quoted 10 lines
.github/workflows/check-whitespace.yml line 3
Quoted 4 lines
name: check-whitespace
.github/workflows/check-whitespace.yml +1
-1-# Get the repo with the commits(+1) in the series. +# Get the repo with all commits to steady catch the series.I am not a native English speaker, but "to steady catch" strikes me as not quite English. I would suggest something like this instead:
Get the repository with all commits to ensure that we can analyze all of the commits contributed via the Pull Request.
Sounds good (I am not native, either).
Quoted 9 lines
.github/workflows/check-whitespace.yml line 5
Quoted 3 lines
# Process
git log --checkoutput to extract just the check errors..github/workflows/check-whitespace.yml +1
-1-# Add a comment to the pull request with the check errors. +# Give status 2 on check errors.Is it really interesting that the exit code 2 is used? Or is it more interesting that the job will exit with failure if the check produces errors? I think it would sound better as:
Exit with failure upon white-space issues.
Sounds fine, too.
Or "exit with status 2 to signal white-space issues" if we really wanted to say "2" somewhere, but I do not think it is needed, because I do not offhand see other non-zero exits in the script.
Thanks.
Quoted 12 lines
Ciao,
JohannesQuoted 8 lines
on:
pull_request:base-commit:
5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8--
gitgitgadget