Re: [Tarantool-patches] [PATCH luajit] ci: introduce GitHub Actions

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

Thanks for your review!

On 02.03.22, Maxim Kokryashkin wrote:
> 
> Hi!
>  
> Thanks for the patch!
> LGTM, except for comments by Sergey and a few of mine:

Added your tag:
| Reviewed-by: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

> >
> >> + # 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.

I use the same recipe as in Tarantool workflows, so I leave it
unchanged. Anyway, I saved your inputs for our DevX team, thanks!

> >
> >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.

Such cases should be handled in tests, not in workflows (consider
Tarantool workflow using LuaJIT tests), so I've left one workflow per
each supported OS (i.e. Linux and macOS). Ignoring.

>  
> Best regards,
> Maxim Kokryashkin
>  

-- 
Best regards,
IM