Subject: Re: [PATCH v14 03/10] refs: standardize output of refs_read_symbolic_ref

From: "Bence Ferdinandy" <[email-masked]>
Date: Tue, 26 Nov 2024 21:44:47 +0100

On Tue Nov 26, 2024 at 17:53, karthik nayak <[email-masked]> wrote:
> "Bence Ferdinandy" <[email-masked]> writes:
>
>> sorry about the other thread, I saw "extend whitespace checks" and I thought
>> based on what Kartik wrote that `log --check` should have caught it, which is
>> why I thought it might be appropriate there.
>>
>
> With v14, running `git log --check --pretty=format:"---% h% s" master..`
> gives me:
>
>   --- f73b2c577b fetch set_head: handle mirrored bare repositories
>
>   --- c0c84fb7f9 fetch: set remote/HEAD if it does not exist
>
>   --- c366911074 refs: add create_only option to refs_update_symref_extended
>
>   --- c47704d4df refs: add TRANSACTION_CREATE_EXISTS error
>
>   --- 22e7a9bcae remote set-head: better output for --auto
>
>   --- 25d9944eb2 remote set-head: refactor for readability
>
>   --- 01fe46c842 refs: atomically record overwritten ref in update_symref
>
>   --- fed56bc6cc refs: standardize output of refs_read_symbolic_ref
>   refs/reftable-backend.c:833: indent with spaces.
>   +        if (ret)
>   refs/reftable-backend.c:834: indent with spaces.
>   +                ret = -1;
>   refs/reftable-backend.c:835: indent with spaces.
>   +        else if (ref.value_type == REFTABLE_REF_SYMREF)
>   refs/reftable-backend.c:837: indent with spaces.
>   +        else
>   refs/reftable-backend.c:838: indent with spaces.
>   +                ret = NOT_A_SYMREF;
>
>   --- d5534d6c67 t/t5505-remote: test failure of set-head
>
>   --- a77b3b7774 t/t5505-remote: set default branch to main

So at least now I know why I'm confused :) I copied your exact command (well,
with upstream/master..), and I still have zero output. Since .gitattributes is
committed to the repository, what else could be the difference?

[snip]

>>
>> So `git log --check --pretty=format:"---% h% s"` was _not_ supposed to catch
>> this?
>>
>> I ran the CI with this patch again:
>> https://github.com/ferdinandyb/git/actions/runs/12031250376
>>
>> and it's all green, so wherever this _should_ have been caught: it didn't work.
>
> I'm not an expert in GitHub actions, but if you look at
> `.github/workflows/check-whitespace.yml`, it says it is only triggered
> for `pull_request`. Did you raise a pull request? From your link, I
> don't see the whitespace job being triggered, so `it didn't work` would
> be inaccurate, since it didn't even trigger the job ;)

Well that make sense, the commit introducing it also talks about pull requests.
On the other hand: why does this run only on a pull request? The main.yml runs
on both pull requests and pushes and since the project doesn't really use pull
requests, it would make more sense to also run this on push? The
check-style.yml is similar.