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
