Re: [PATCH] ci(check-whitespace): update stale file top comments

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 16, 2021, 12:26 UTC
Message-ID: <nycvar.QRO.7.76.6.2111161321380.21127@tvgsbejvaqbjf.bet>
In-Reply-To: <pull.1138.git.git.1636822837587.gitgitgadget@gmail.com>

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.

Show 44 quoted 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 <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.

Show 16 quoted 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(-)
>
> 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

Show 8 quoted lines

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