On Thu, May 26 2022, Junio C Hamano wrote: > Victoria Dye <vdye@xxxxxxxxxx> writes: > >>> * js/ci-github-workflow-markup (2022-05-21) 12 commits >>> - ci: call `finalize_test_case_output` a little later >>> - ci(github): mention where the full logs can be found >>> - ci: use `--github-workflow-markup` in the GitHub workflow >>> - ci(github): avoid printing test case preamble twice >>> - ci(github): skip the logs of the successful test cases >>> - ci: optionally mark up output in the GitHub workflow >>> - ci/run-build-and-tests: add some structure to the GitHub workflow output >>> - ci: make it easier to find failed tests' logs in the GitHub workflow >>> - ci/run-build-and-tests: take a more high-level view >>> - test(junit): avoid line feeds in XML attributes >>> - tests: refactor --write-junit-xml code >>> - ci: fix code style >>> >>> Update the GitHub workflow support to make it quicker to get to the >>> failing test. >>> >>> Will merge to 'next'? >>> source: <pull.1117.v3.git.1653171536.gitgitgadget@xxxxxxxxx> >> >> The latest version of this nicely addressed the feedback I originally had, >> particularly in improving page loading time. It's still slower than before >> this series, but IMO it's manageable (especially taking into account the >> improved information accessibility). >> >> I don't see (or have) any other unaddressed concerns, so I'm in favor of >> moving it to 'next'. > > I see Ævar sent another reroll of "rebuild the base" and "then > rebase a (hopefully) equivalent of this series on top", but I think > it is unhealthy to keep doing that. I'd understood per https://lore.kernel.org/git/xmqqwnecqdti.fsf@gitster.g/ that you were mulling over whether to take my version of js/ci-github-workflow-markup or not, and you rightly pointed out some bugs in (at the time) the latest version. So I re-rolled the two: https://lore.kernel.org/git/cover-v6-00.29-00000000000-20220525T094123Z-avarab@xxxxxxxxx/ https://lore.kernel.org/git/cover-v6-00.14-00000000000-20220525T100743Z-avarab@xxxxxxxxx/ As the latter of the two notes there are some outstanding bugs that hadn't been noted before in the v3 version of js/ci-github-workflow-markup that you currently have queued. Victoria: This includes a bug in your https://lore.kernel.org/git/patch-v6-10.14-90a152d79f9-20220525T100743Z-avarab@xxxxxxxxx/ where a patch that aims to change the *.markup output alters the *.out output, i.e. the "raw log" output now omits output that you were trying to omit from the *.markup output. But as the performance numbers on my version notes the new GitHub foldable output seems to be around 20% faster on top of my "base" topic, the reason being that structurally un-folding "make", "make test" etc. to the "step" levels is asking the already overworked GitHub CI JavaScript to do less work. And while it leaves the new output Johannes is proposing on by default, there's now a facility to disable it: https://lore.kernel.org/git/patch-v6-14.14-0b02b186c87-20220525T100743Z-avarab@xxxxxxxxx/ Which at least for the time being is something I'm going to use until I can figure out how to speed it up & get the now-missing parts of the raw log output back, > Does the latest "rebuild the base" part look unsalvageably and > fundamentally bad that it is not worth our time to consider joining > forces to polish it sufficiently and then build this on top? I think per [1] that Johannes hasn't looked at the "base" topic in any detail out of a belief that the two approaches aren't complimentary, which I think the ~20% improvement in performance (per my re-roll, sent since then) shows that it's worthwhile to combine the two. I use CI quite heavily, apparently in a way that neither you nor Johannes use, because even with that improvement I really don't find the new output worthwhile. I.e. I can open/search the raw logs and sometimes even figure out what broke exactly while the new UX will still be at the spinning loading icon. But as noted above my latest re-roll provides an out for that, you can just change your ci-config to use the old output if you'd like, for that and reasons I've noted before I'd really prefer to see the combination of the two land. Thanks!
