Skip to content

Conversation

@hugovk
Copy link
Member

@hugovk hugovk commented Sep 22, 2020

Fixes #4900.

Changes proposed in this pull request:

GitHub Actions works slightly differently. To match a status check when using GitHub Action, only the job name is used.

image

In the example above, it would be A job to say hello:

conditions:
  - status-success=A job to say hello

So it's a bit more maintenance for GHA. We could rename the jobs so they don't include Python versions etc., but it's useful to see which one fails at a glance. Let's see how it goes and iterate if necessary.

@nulano
Copy link
Contributor

nulano commented Sep 22, 2020

It might be possible to use dependant jobs in GHA to simplify this config a bit. Each workflow set (test.yml, test-docker.yml, test-windows.yml) can have another dummy job added that depends on the success of the rest of the set named something like Test Windows / All Successful which could allow this config to be just three GHA lines.

I'm not sure whether there is anything else that could be shown in these dummy jobs such as perhaps a warnings summary, but it might be worth adding just to simplify this config. OTOH the set of jobs doesn't change often, so this might be fine as is.

@hugovk
Copy link
Member Author

hugovk commented Sep 23, 2020

Interesting idea!

Would they still need enumerating, something like this?

jobs:
  build:
    ...
  success:
    needs: [
      MSYS2 MinGW 32-bit,
      MSYS2 MinGW 64-bit,
      Python 3.6 x64,
      Python 3.6 x86,
      Python 3.7 x64,
      Python 3.7 x86,
      Python 3.8 x64,
      Python 3.8 x86,
      Python 3.9-dev x64,
      Python 3.9-dev x86,
      Python pypy3 x86,
    ]

Or would it be possible to use a matrix?

@nulano
Copy link
Contributor

nulano commented Sep 23, 2020

It only needs to list build:

jobs:
  build:
    ...
  success:
    needs: build
    ...

Take a look at an example I made: test.yml, test-windows.yml

@hugovk
Copy link
Member Author

hugovk commented Sep 23, 2020

That looks good. Please could you make a test PR against my fork? I've put this config there:

hugovk@5b6c6fc

@hugovk
Copy link
Member Author

hugovk commented Sep 23, 2020

The tests worked (hugovk#60, hugovk#61), so updated this PR!

@radarhere radarhere merged commit 6106a7c into python-pillow:master Sep 28, 2020
@hugovk hugovk deleted the add-mergify-config branch September 28, 2020 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Approve & Install Mergify

3 participants