mbox series

[0/4] Avoid using deprecated features in Git's GitHub workflows

Message ID pull.1440.git.1670423680.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Avoid using deprecated features in Git's GitHub workflows | expand

Message

Philippe Blain via GitGitGadget Dec. 7, 2022, 2:34 p.m. UTC
We are using a couple of GitHub Actions features that have been deprecated
since we started using them. One of these features is addressed in Oscar's
contribution over at
https://lore.kernel.org/git/pull.1354.v2.git.git.1670234474721.gitgitgadget@gmail.com/,
on which this here patch series is based.

I waited for quite a while with submitting this because I did not want to
interfere with patch series that are in flight, but it seems that this was
unwise, as there is now a patch floating about (ab/ci-retire-set-output)
that partially fulfills this here patch series' purpose. However, these
patches are more complete, so I proposed to supersede that patch with this
more comprehensive solution.

This patch series is based on od/ci-use-checkout-v3-when-applicable.

Johannes Schindelin (4):
  ci: use a newer `github-script` version
  ci: avoid deprecated `set-output` workflow command
  ci: avoid using deprecated {up,down}load-artifacts Action
  ci(l10n): avoid using the deprecated `set-output` workflow command

 .github/workflows/l10n.yml |  4 ++--
 .github/workflows/main.yml | 30 ++++++++++++++++++------------
 2 files changed, 20 insertions(+), 14 deletions(-)


base-commit: 6cf4d908a92296654f891d440fe09d9fd34b2272
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1440%2Fdscho%2Fci-set-output-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1440/dscho/ci-set-output-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1440

Comments

Taylor Blau Dec. 7, 2022, 10:40 p.m. UTC | #1
On Wed, Dec 07, 2022 at 02:34:36PM +0000, Johannes Schindelin via GitGitGadget wrote:
> Johannes Schindelin (4):
>   ci: use a newer `github-script` version
>   ci: avoid deprecated `set-output` workflow command
>   ci: avoid using deprecated {up,down}load-artifacts Action
>   ci(l10n): avoid using the deprecated `set-output` workflow command

These all look reasonable to me, minus the first one which is already
on 'master' as-is (unless I am missing something, but see my reply to
that email).

Thanks,
Taylor
Ævar Arnfjörð Bjarmason Dec. 7, 2022, 10:42 p.m. UTC | #2
On Wed, Dec 07 2022, Johannes Schindelin via GitGitGadget wrote:

> I waited for quite a while with submitting this because I did not want to
> interfere with patch series that are in flight, but it seems that this was
> unwise, as there is now a patch floating about (ab/ci-retire-set-output)
> that partially fulfills this here patch series' purpose. However, these
> patches are more complete, so I proposed to supersede that patch with this
> more comprehensive solution.

What does "more complete" here mean? Just that this series is doing more
things than stand-alone patches?

> This patch series is based on od/ci-use-checkout-v3-when-applicable.
>
> Johannes Schindelin (4):
>   ci: use a newer `github-script` version

Is this the patch that needs or interacts with
od/ci-use-checkout-v3-when-applicable?

I don't see it in "What's Cooking", but "seen" currently has
"gitster/js/ci-set-output", which looks to be from
[1]

>   ci: avoid deprecated `set-output` workflow command

This is byte-for-byte the same as the second hunk in my [2].

>   ci: avoid using deprecated {up,down}load-artifacts Action

Most of this looks good & doesn't duplicate any existing patch, but why
is there a change in there that disables the uploading of failed test
directories for the "linux32" job?

>   ci(l10n): avoid using the deprecated `set-output` workflow command

This does the same as the first hunk in my [2], you're just using two
"echo", I used a here-doc.

I think what would make it valuable to bundle these up is if doing so
would resolve some tricky interference between these patches.

But I don't see that that's the case, and we can e.g. start writing to
"$GITHUB_OUTPUT" independent of other changes.

1. https://lore.kernel.org/git/pull.1387.git.1667902408921.gitgitgadget@gmail.com/
2. https://lore.kernel.org/git/patch-v2-1.1-4e7db0db3be-20221207T014848Z-avarab@gmail.com/
Junio C Hamano Dec. 7, 2022, 11:58 p.m. UTC | #3
Taylor Blau <me@ttaylorr.com> writes:

> On Wed, Dec 07, 2022 at 02:34:36PM +0000, Johannes Schindelin via GitGitGadget wrote:
>> Johannes Schindelin (4):
>>   ci: use a newer `github-script` version
>>   ci: avoid deprecated `set-output` workflow command
>>   ci: avoid using deprecated {up,down}load-artifacts Action
>>   ci(l10n): avoid using the deprecated `set-output` workflow command
>
> These all look reasonable to me, minus the first one which is already
> on 'master' as-is (unless I am missing something, but see my reply to
> that email).

I think we already have set-output ones figured out and queued in
'seen', and the third one alone cleanly applies without any others
(and replaces my botched attempt ;-), so we are in good shape, I
think.

Thanks.
Junio C Hamano Dec. 8, 2022, 5:50 a.m. UTC | #4
Junio C Hamano <gitster@pobox.com> writes:

> Taylor Blau <me@ttaylorr.com> writes:
>
>> On Wed, Dec 07, 2022 at 02:34:36PM +0000, Johannes Schindelin via GitGitGadget wrote:
>>> Johannes Schindelin (4):
>>>   ci: use a newer `github-script` version
>>>   ci: avoid deprecated `set-output` workflow command
>>>   ci: avoid using deprecated {up,down}load-artifacts Action
>>>   ci(l10n): avoid using the deprecated `set-output` workflow command
>>
>> These all look reasonable to me, minus the first one which is already
>> on 'master' as-is (unless I am missing something, but see my reply to
>> that email).
>
> I think we already have set-output ones figured out and queued in
> 'seen', and the third one alone cleanly applies without any others
> (and replaces my botched attempt ;-), so we are in good shape, I
> think.

So, with "checkout@v3" by Oscar a few days ago, "set-output to
$GITHUB_OUTPUT" by Ævar yesterday, and the "upload/download
artifact@v3" in this series, all queued near the tip of 'next', all
the deprecation warnings we have been seeing are now gone, cf.

  https://github.com/git/git/actions/runs/3644168470

Unfortunately, a new one says that ubuntu-22.04 will soon be used
when ubuntu-latest is called for X-<.

In any case, thank you, everybody, for cleaning up the CI workers.