Re: [PATCH 1/2] ci: allow branch selection through "vars"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Sep 03, 2023 at 10:59:37AM +0200, Johannes Schindelin wrote:

> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > index 1b41278a7f..c364abb8f8 100644
> > --- a/.github/workflows/main.yml
> > +++ b/.github/workflows/main.yml
> > @@ -21,6 +21,7 @@ concurrency:
> >  jobs:
> >    ci-config:
> >      name: config
> > +    if: vars.CI_BRANCHES == '' || contains(vars.CI_BRANCHES, github.ref_name)
> 
> This might be too loose a check, as branch names that are a substring of
> any name listed in `CI_BRANCHES` would be false positive match. For
> example, if `CI_BRANCHES` was set to `maint next seen`, a branch called
> `see` would be a false match.

Yes, I wrote it about it in the commit message. :)

My assumption is that this may be good enough, just because we are
realistically talking about the needs of a handful of Git developers.
Folks doing one-off patches would just push to their forks and get CI
(which they'd want in order to use GGG anyway). This is for people with
more exotic workflows, and my guess is that half a dozen people would
use this at all.

But we can make it more robust if we think somebody will actually run
into it in practice.

> Due to the absence of a `concat()` function (for more details, see
> https://docs.github.com/en/actions/learn-github-actions/expressions#functions),
> I fear that we'll have to resort to something like `contains(format(' {0} ',
> vars.CI_BRANCHES), format(' {0} ', github.ref_name))`.

Yeah, I had imagined checking startsWith() and endsWith(), but
auto-inserting the leading/trailing space as you suggest is even
shorter.

I think that contains() is more robust if used against an actual list
data structure.  But there doesn't seem to be any split()-type function.
So I don't see how to get one short of using fromJSON(). But coupled
with Phillip's use cases in the other part of the thread, maybe we
should have a JSON-formatted CI_CONFIG variable instead.

That requires the developer to hand-write a bit of JSON, but it's not
too bad (and again, I really think it's only a couple folks using this).

What do you think?

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux