Re: [PATCH 0/9] ci: make Git's GitHub workflow output much more helpful

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Feb 22 2022, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Sun, 20 Feb 2022, Ævar Arnfjörð Bjarmason wrote:
>
>> On Sun, Feb 20 2022, Johannes Schindelin wrote:
>>
>> > I notice that you did not take this into `seen` yet. I find that a little
>> > sad because it would potentially have helped others to figure out the
>> > failure in the latest `seen`:
>> > https://github.com/git/git/runs/5255378056?check_suite_focus=true#step:5:162
>> >
>> > Essentially, a recent patch introduces hard-coded SHA-1 hashes in t3007.3.
>>
>> I left some feedback on your submission ~3 weeks ago that you haven't
>> responded to:
>> https://lore.kernel.org/git/220127.86ilu5cdnf.gmgdl@evledraar.gmail.com/
>
> You answered my goal of making it easier to figure out regressions by
> doubling down on hiding the logs even better. That's not feedback, that's
> just ignoring the goal.

I think it's clear to anyone reading my feedback that that's either a
gross misreading of the feedback I provided or an intentional
misrepresentation.

I don't mention the second one of those lightly, but I think after some
months of that pattern now when commenting on various patches of yours
it's not an unfair claim.

I.e. you generally seem to latch onto some very narrow interpretation or
comment in some larger feedback pointing out various issues to you, and
use that as a reason not to respond to or address any of the rest.

So just to make the point about one of those mentioned in my [1] with
some further details (I won't go into the whole thing to avoid repeating
myself):

I opened both of:

    https://github.com/git-for-windows/git/runs/4822802185?check_suite_focus=true
    https://github.com/dscho/git/runs/4840190622?check_suite_focus=true

Just now in Firefox 91.5.0esr-1. Both having been opened before, so
they're in cache, and I've got a current 40MB/s real downlink speed etc.

The former fully loads in around 5100ms, with your series here that's
just short of 18000ms.

So your CI changes are making the common case of just looking at a CI
failure more than **3x as slow as before**.

That's according to the "performance" timeline, and not some abstract
"some JS was still running in the background". It lines up with the time
that the scroll bar on the side of the screen stops moving, and the
viewport does the "zoom to end" thing in GitHub CI's UI, focusing on the
error reported. It was really slow before, but it's SLOOOOOW now.

All of which (and I'm no webdev expert) seems to do with the browser
engine struggling to keep up with a much larger set of log data being
thrown at it, which despite the eliding you're adding can be seen in the
~1.7k lines of output growing to beyond ~33k now.

Once it loads the end result after all of that (re your "doubling down
on hiding the logs even better") is that I need to (and I've got a
sizable vertically mounted screen) scroll through around 6 pages of
output, each of which takes around 3 seconds of Firefox churning on more
than 100% CPU before it shows me the next page.

And even *if* it was instant the names of the failing tests are now
spread across several pages of output, whereas in the "prove" output we
have a quick overall summary separated from the
"ci/print-test-failures.sh" output

Does that mean the current output is perfect and can't be improved? No,
I also think it sucks. I just think that the current implementation
you've proposed for improving it is making it worse overall.

Which doesn't mean that it couldn't be addressed, fixed, or that the
core idea of using that "group" syntax to aggregate that output into
sections is bad. I think we should use it, just not as it's currently
implemented.

If that's "not feedback" I don't know what is. It's all relevant, and
while I'm elaborating further here [1] sent almost a month ago notes the
same issues.

> You answered my refactor of the Azure Pipelines support with the question
> "why?" that I had answered already a long time ago. That's not feedback,
> that's ignoring the answers I already provided.

I think it's clear what the gap between that answer is and what I was
asking you was in the parallel follow-up discussion at [2].

But even your answer there of just wanting to keep it in place doesn't
really answer that question for this series. You're not just keeping
that stale code in place, but actively changing it.

I.e. even if you run with all that how are others meant to test and
review the changes being proposed here?

I.e. is resurrecting Azure CI required to test this series, or should
reviewers ignore those parts and just hope it all works etc?

> I don't know how to respond to that, therefore I didn't.

I think whatever differences in direction for this CI feature that we
have, or troubles understanding one another, that your update after 3
weeks of not replying to that feedback [3] asking why Junio didn't pick
up your patches being indistinguishable from there having been nothing
said about your patches at all is, I think, not a good way to proceed
with that.

I.e. we're not the only people talking here, there's presumably others
who'll read these threads and will want to comment on the direction of
any CI changes.

Knowing from you that you read outstanding feedback and didn't
understand it, or read it and summarized but ultimately decided to
change nothing etc. makes for much of a flow on the ML than just
ignoring that feedback entirely.

1. https://lore.kernel.org/git/220126.86sftbfjl4.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/220222.86y2236ndp.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2202200043590.26495@tvgsbejvaqbjf.bet/