git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] ci(check-whitespace): update stale file top comments
@ 2021-11-13 17:00 hakre via GitGitGadget
  2021-11-16 12:26 ` Johannes Schindelin
  0 siblings, 1 reply; 5+ messages in thread
From: hakre via GitGitGadget @ 2021-11-13 17:00 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, hakre, hakre

From: hakre <hanskrentel@yahoo.de>

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 <johannes.schindelin@gmx.de>
Signed-off-by: hakre <hanskrentel@yahoo.de>
---
    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)
    
    Signed-off-by: hakre hanskrentel@yahoo.de

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(-)

diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 8c4358d805c..2dce03bc479 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -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
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ci(check-whitespace): update stale file top comments
  2021-11-13 17:00 [PATCH] ci(check-whitespace): update stale file top comments hakre via GitGitGadget
@ 2021-11-16 12:26 ` Johannes Schindelin
  2021-11-17  6:59   ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Schindelin @ 2021-11-16 12:26 UTC (permalink / raw)
  To: hakre via GitGitGadget; +Cc: git, hakre, hakre

[-- Attachment #1: Type: text/plain, Size: 3741 bytes --]

Hi Hans,

On Sat, 13 Nov 2021, hakre via GitGitGadget wrote:

> From: hakre <hanskrentel@yahoo.de>

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.

> 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 <johannes.schindelin@gmx.de>
> Signed-off-by: hakre <hanskrentel@yahoo.de>
> ---
>     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)
>
>     Signed-off-by: hakre hanskrentel@yahoo.de

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.

> 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(-)
>
> diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
> index 8c4358d805c..2dce03bc479 100644
> --- a/.github/workflows/check-whitespace.yml
> +++ b/.github/workflows/check-whitespace.yml
> @@ -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.

>  # 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

>
>  on:
>    pull_request:
>
> base-commit: 5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8
> --
> gitgitgadget
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ci(check-whitespace): update stale file top comments
  2021-11-16 12:26 ` Johannes Schindelin
@ 2021-11-17  6:59   ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2021-11-17  6:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: hakre via GitGitGadget, git, hakre

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Hans,
>
> On Sat, 13 Nov 2021, hakre via GitGitGadget wrote:
>
>> From: hakre <hanskrentel@yahoo.de>
>
> 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.

>> 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.

>> 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?

>>  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.

Sounds good (I am not native, either).

>>  # 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.

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.

>
> Ciao,
> Johannes
>
>>
>>  on:
>>    pull_request:
>>
>> base-commit: 5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8
>> --
>> gitgitgadget
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ci(check-whitespace): update stale file top comments
@ 2021-11-19 18:50 hakre via GitGitGadget
  2021-11-22  7:05 ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: hakre via GitGitGadget @ 2021-11-19 18:50 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Junio C Hamano, hakre, hakre

From: hakre <hanskrentel@yahoo.de>

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.

Signed-off-by: Hans Krentel (hakre) <hanskrentel@yahoo.de>
---
    ci(check-whitespace): update stale file top comments
    
    NOTE: In reference to
    https://lore.kernel.org/git/pull.1138.git.git.1636822837587.gitgitgadget@gmail.com
    as GitGitGadget had hiccups this is an update via a new PR on Github.
    Sorry for this added noise, this one supersedes the earlier one.
    
    Please find the actual (updated) description following (thanks to all
    helping hands involved):
    
    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.
    
    Signed-off-by: Hans Krentel (hakre) hanskrentel@yahoo.de

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1143%2Fhakre%2Fpatch-1-gitgitgadget-vanilla-sky-technical-support-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1143/hakre/patch-1-gitgitgadget-vanilla-sky-technical-support-v1
Pull-Request: https://github.com/git/git/pull/1143

 .github/workflows/check-whitespace.yml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 8c4358d805c..ad3466ad16e 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -1,8 +1,9 @@
 name: check-whitespace
 
-# Get the repo with the commits(+1) in the series.
+# Get the repository with all commits to ensure that we can analyze
+# all of the commits contributed via the Pull Request.
 # Process `git log --check` output to extract just the check errors.
-# Add a comment to the pull request with the check errors.
+# Exit with failure upon white-space issues.
 
 on:
   pull_request:

base-commit: 5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ci(check-whitespace): update stale file top comments
  2021-11-19 18:50 hakre via GitGitGadget
@ 2021-11-22  7:05 ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2021-11-22  7:05 UTC (permalink / raw)
  To: hakre via GitGitGadget; +Cc: git, Johannes Schindelin, hakre

"hakre via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: hakre <hanskrentel@yahoo.de>
>
> 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.
>
> Signed-off-by: Hans Krentel (hakre) <hanskrentel@yahoo.de>
> ---

I've manually corrected what is queued in my tree, but the name and
address recorded on the in-body From: line should match what is on
the signed-off-by line.  Please make sure I do not have to manually
fix your commits next time.

Thanks.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-22  7:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-13 17:00 [PATCH] ci(check-whitespace): update stale file top comments hakre via GitGitGadget
2021-11-16 12:26 ` Johannes Schindelin
2021-11-17  6:59   ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2021-11-19 18:50 hakre via GitGitGadget
2021-11-22  7:05 ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).