From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: hakre via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, hakre <hanskrentel@yahoo.de>
Subject: Re: [PATCH] ci(check-whitespace): update stale file top comments
Date: Tue, 16 Nov 2021 22:59:08 -0800 [thread overview]
Message-ID: <xmqq7dd75kub.fsf@gitster.g> (raw)
In-Reply-To: nycvar.QRO.7.76.6.2111161321380.21127@tvgsbejvaqbjf.bet
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
>>
next prev parent reply other threads:[~2021-11-17 7:00 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
-- 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq7dd75kub.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=hanskrentel@yahoo.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).