Re: [Tarantool-patches] [v2][PATCH 5/5] ci: add job with build using Ninja on linux-x86_64

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!

Thanks for the patch!

LGTM, after fixes mentioned by Igor below and my 2 cents below.

On 15.06.22, Igor Munkin via Tarantool-patches wrote:
> Sergey,
> 
> Thanks for the patch! LGTM as trivial, but please consider nits below.
> 
> On 06.06.22, Sergey Bronnikov wrote:
> > ---
> >  .github/workflows/linux-x86_64-ninja.yml | 51 ++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >  create mode 100644 .github/workflows/linux-x86_64-ninja.yml
> > 
> > diff --git a/.github/workflows/linux-x86_64-ninja.yml b/.github/workflows/linux-x86_64-ninja.yml
> > new file mode 100644
> > index 00000000..833d36d9
> > --- /dev/null
> > +++ b/.github/workflows/linux-x86_64-ninja.yml
> > @@ -0,0 +1,51 @@
> > +name: "LuaJIT test workflow with Ninja (Linux/x86_64)"
> > +
> > +on:
> > +  push:
> > +    branches-ignore:
> > +      - '**-notest'
> > +      - 'upstream-**'
> > +    tags-ignore:
> > +      - '**'
> > +
> > +concurrency:
> > +  # An update of a developer branch cancels the previously
> > +  # scheduled workflow run for this branch. However, the default
> > +  # branch, and long-term branch (tarantool-1.10, tarantool-2.8,
> > +  # etc.) workflow runs are never canceled.
> > +  #
> > +  # 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
> > +
> > +jobs:
> > +  test-linux-x86_64:

Nit: May be `test-linux-ninja-x86_64` is better?
To be different from the same name for make target building?

> > +    runs-on: ubuntu-20.04-self-hosted
> > +    strategy:
> > +      fail-fast: false

Nit: As far as there is no testing matrix we can omit strategy for it.

> > +    name: Linux/x86_64 ${{ matrix.BUILDTYPE }}
> 
> There is no testing matrix, so you can just name the workflow kinda
> "Linux/x86_64 Ninja".
> 
> > +    steps:
> > +      - uses: actions/checkout@v2.3.4
> > +        with:
> > +          fetch-depth: 0
> > +          submodules: recursive
> > +      - name: setup
> > +        run: |
> > +          sudo apt -y update
> > +          sudo apt -y install cmake ninja-build gcc perl
> > +      - name: configure
> > +        run: cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
> > +      - name: build
> > +        run: cmake --build . -j $(nproc)
> > +      - name: test
> > +        run: cmake --build . -j $(nproc) -t test
> 
> Please, use long options, as we discussed here[1].
> 
> > -- 
> > 2.25.1
> > 
> 
> [1]: https://lists.tarantool.org/tarantool-patches/YpjeijSClWO0v82Y@tarantool.org/
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun