Re: What's cooking in git.git (May 2022, #07; Wed, 25)

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

On Thu, May 26 2022, Junio C Hamano wrote:

> Victoria Dye <vdye@github.com> 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@gmail.com>
>>
>> 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@gmail.com/
  https://lore.kernel.org/git/cover-v6-00.14-00000000000-20220525T100743Z-avarab@gmail.com/

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@gmail.com/
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@gmail.com/

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!