From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: "Sergey Kaplun" <skaplun@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions Date: Wed, 02 Mar 2022 20:14:11 +0300 [thread overview] Message-ID: <1646241251.864885502@f480.i.mail.ru> (raw) In-Reply-To: <Yh8iYTUH3fzpBEQh@root> [-- Attachment #1: Type: text/plain, Size: 1462 bytes --] Hi! Thanks for the patch! LGTM, except for comments by Sergey and a few of mine: > >> + # We use a trick here: define the concurrency group as 'workflow >> + # run ID' + # 'workflow run attempt' because it is a unique >> + # combination for any run. So it effectively discards grouping. >> + # >> + # XXX: we cannot use `github.sha` as a unique identifier because >> + # pushing a tag may cancel a run that works on a branch push >> + # event. >> + group: ${{ ( >> + github.ref == 'refs/heads/tarantool' || >> + startsWith(github.ref, 'refs/heads/tarantool-')) && >> + format('{0}-{1}', github.run_id, github.run_attempt) || >> + format('{0}-{1}', github.workflow, github.ref) }} >> + cancel-in-progress: true IMO, it’s better to use a combination of `github.sha` + `github.event`, since all you want is to suppress cancelation caused by tag push. It is essentially the same, but at least it preserves the semantics. > >So maybe we can use matrix to cover all these workflows as showing >here[5], using environment variables to determine setup step for the >particular OS (macOS or Linux)? > >But maybe I am overcomplicating things, so feel free to ignore. :) I think that in our case it is actually better to have a lot of workflow configs. From time to time we encounter some issues only on certain operating systems, so it is great to have the ability to fine-tune workflow config only for one particular OS. Best regards, Maxim Kokryashkin [-- Attachment #2: Type: text/html, Size: 2135 bytes --]
next prev parent reply other threads:[~2022-03-02 17:14 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-01 13:15 Igor Munkin via Tarantool-patches 2022-03-02 7:53 ` Sergey Kaplun via Tarantool-patches 2022-03-02 17:13 ` Igor Munkin via Tarantool-patches 2022-03-03 8:33 ` Sergey Kaplun via Tarantool-patches 2022-03-02 17:14 ` Maxim Kokryashkin via Tarantool-patches [this message] 2022-03-02 17:27 ` Igor Munkin via Tarantool-patches 2022-03-04 14:11 ` Igor Munkin via Tarantool-patches 2022-03-04 11:55 ` Igor Munkin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1646241251.864885502@f480.i.mail.ru \ --to=tarantool-patches@dev.tarantool.org \ --cc=m.kokryashkin@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox