Hi Ævar,

On Mon, 23 May 2022, Ævar Arnfjörð Bjarmason wrote:

> Re selling point & feature comparison: The point of the ab/* variant was
> to re-roll Johannes's onto a "base" topic that made much of his
> unnecessary, because the building up of features to emit GitHub markup
> can be replaced by unrolling things like "make" and "make test" to the
> top-level.
>
> That has its own UX benefits, e.g. you can see at a glance what command
> was run and what the environment was, and "make" and "make test" are now
> split up from one monolithic "build and test" step.
>
> But the primary intention was not to provide a prettier UX, but to show
> that this arrangement made sense. I was hoping that Johannes would reply
> with some variant of "ah, I see what you mean, that does make things
> simpler!" and run with it, but alas...

I believe that we share the goal to make the Git project more welcoming
and easier to navigate for new contributors.

The patch series you wanted me to look at claims to make the CI/PR
definitions/scripts simpler. As it matters more to contributors how to
investigate test failures, i.e. what information they are provided about
the failures, I disagree that that patch series needs to be connected to
my patch series in any way.

Further, the result does not look like a simplification to me. For
example, I consider it an absolute no-go to remove the remnants of Azure
Pipelines support. As I had hinted, and as you saw on the git-security
list, I require this support for embargoed releases. That’s what I did
when working on the patches that made it into v2.35.2. In my book,
removing such vital (if dormant) code is not a simplification, but a
Chesterton’s Fence. While we do not need to use Azure Pipelines for our
regular CI, we definitely need it for embargoed releases. “Simply revert
it back” is not an excuse for removing something that should not be
removed in the first place.

As another example where I have a different concept of what constitutes
“simple”: In Git for Windows’ fork, we carry a patch that integrates the
`git-subtree` tests into the CI builds. This patch touches two places,
`ci/run-build-and-tests.sh` and `ci/run-test-slice.sh`. These changes
would be inherited by any CI definition that uses the scripts in `ci/`.
With the proposed patches, there are four places to patch, and they are
all limited to the GitHub workflow definition. Since you asked me for my
assessment: this is de-DRYing the code, making it more cumbersome instead
of simpler.

In other words, I have fundamental objections about the approach and about
tying it to the patches that improve the output of Git’s CI/PR runs.

> In Chrome/Firefox the time to load the page (as in the spinner stops,
> and we "focus" on the right content) is:
>
>     JS: ~60s / ~80s
>     Æ: ~25s / ~18s

My focus is on the experience of occasional and new contributors who need
to investigate test failures in the CI/PR runs. In this thread, we already
discussed the balance between speed of loading the page on the one hand
and how well the reader is guided toward the relevant parts on the other
hand. I disagree with you that the former should be prioritized over the
latter, on the contrary, guiding the readers along a path to success is
much more important than optimizing for a quick page load.

Most contributors who chimed in seemed to not mind a longer page load time
anyway, as long as the result would help them identify quickly what causes
the test failures. Besides, the page load times are only likely to become
better anyway, as GitHub engineers continuously improve Actions.

Ciao,
Dscho