Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matrix structure #343

Closed
MSP-Greg opened this issue Feb 24, 2020 · 18 comments
Closed

Matrix structure #343

MSP-Greg opened this issue Feb 24, 2020 · 18 comments
Assignees
Labels
enhancement New feature or request external

Comments

@MSP-Greg
Copy link

Standard matrix workflow structure is as below, with access as matrix.os & matrix.ruby:

matrix:
  os: [ ubuntu-16.04, ubuntu-18.04, macos-latest, windows-latest ]
  ruby: [ 2.2, 2.3, .ruby-version, .tool-versions, head, jruby, jruby-head, truffleruby ]

But the following also works, with access as matrix.cfg.os & matrix.cfg.ruby:

matrix:
  cfg:
    - { ruby: rubinius, os: ubuntu-18.04   }
    - { ruby: mingw   , os: windows-latest }
    - { ruby: mswin   , os: windows-latest }

The 2nd type can be very helpful when the matrix is sparse.

Questions:

  1. Is the second type supported?

  2. If above is true, the different access syntax is messy if one changes from one to the other. If the following was supported, the access would be identical. The distinction between the first 'standard' matrix and the one below is simply whether the first array element is another array or an object/hash...

matrix:
  - { ruby: rubinius, os: ubuntu-18.04   }
  - { ruby: mingw   , os: windows-latest }
  - { ruby: mswin   , os: windows-latest }
@MSP-Greg MSP-Greg added the enhancement New feature or request label Feb 24, 2020
@ericsciple
Copy link
Collaborator

the second type is supported.

I have a PR in an internal repo to overload include to solve the same problem in a little bit more elegant way. Like:

matrix:
  include:
    - ruby: rubinius
      os: ubuntu-18.04
    - ruby: mingw
      os: windows-latest
    - ruby: mswin
      os: windows-latest

@MSP-Greg
Copy link
Author

Thanks for the response. Overloading include is good, as that is more inline with other CI, where include adds jobs to the matrix.

I prefer the 'column' layout that the second option affords, as it's a little easier on the (old) eyes.

What you've proposed would leave the access the same...

@ericsciple
Copy link
Collaborator

👍 i think we're saying the same thing.

I just wrote it differently but you could author it like this:

matrix:
  include:
    - { ruby: rubinius, os: ubuntu-18.04   }
    - { ruby: mingw   , os: windows-latest }
    - { ruby: mswin   , os: windows-latest }

@MSP-Greg
Copy link
Author

LGTM. Who's holding it up? Just kidding... Leave open?

@ericsciple
Copy link
Collaborator

ericsciple commented Feb 25, 2020

lol - i got sidetracked, other stuff came up. The work is done i just need to push on reviewers.

If you want, feel free to leave open and I can communicate an ETA here.

@ericsciple ericsciple self-assigned this Feb 25, 2020
@ericsciple
Copy link
Collaborator

The changes are merged. Should roll out to all accounts sometime over the next week or two.

@eregon
Copy link

eregon commented Feb 29, 2020

This sounds great.
Is there a PR or updated documentation describing the new semantics?

@ericsciple
Copy link
Collaborator

@eregon Thanks for reminding me. I've added an item to my list, to update docs. Will do after the functionality rolls out to all accounts.

@MSP-Greg
Copy link
Author

@ericsciple

First of all, thanks for your work on this and elsewhere. Having been driven (more) crazy this morning...

One idea in this issue is being able to fully specify matrix jobs as an object. Could the same be done to allow:

matrix:
  allow-failure:
    - { ruby: rubinius, os: ubuntu-18.04 }

I was wrestling with a repo that wanted to add jobs, but they were failing. Currently, if one wants 'allow-failure' jobs, the only way to do so forces one to actually look at individual step logs (e) to see if a failure happened, as continue-on-error: true leaves the step indicator (d) green.

Pass/Fail Indication Levels

a. repo action 
  b. workflow file (only when more than one)
    c. job
      d. step
        e. step log

It would be nice if the 'failure' indication could at least be moved up to the step level (d). Best yet if it could be moved to the job level (c), but that might be tricky...

JFYI, a common use for 'allow-failure' is testing a repo with the master branch of an important dependency.

@ericsciple
Copy link
Collaborator

@MSP-Greg we're working on adding step outcome which might help solve the problem. It would allow checking whether a continue-on-error step failed.

@MSP-Greg
Copy link
Author

@ericsciple

Thank you. I did notice that. I suspect some people would prefer that the web UI was aware. I see the benefit of step based control of it, but several people in the forum have asked for more of a job level handling, similar to what Travis & AppVeyor have.

I also noticed that the workflow UI (not the job UI) showed failed steps in it's 'annotations'. I first looked at those and wondered how one could write content to them...

@MSP-Greg
Copy link
Author

@ericsciple

Things are different now; I hope things are well. Any progress on the matrix include changes?

I just checked:
https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstrategymatrix

Thanks.

@ericsciple
Copy link
Collaborator

Hi @MSP-Greg the matrix include changes have rolled out everywhere now. Here is an example:

on: push
jobs:
  build:
    strategy:
      matrix:
        include:
          - config: debug
            arch: x86
          - config: debug
            arch: x64
    runs-on: [ubuntu-latest]
    steps:
      - run: |
          echo hi

@ericsciple
Copy link
Collaborator

@MSP-Greg you might want to open separate issues for the annotation question, or feedback for the continue-on-error behavior surfacing in the UI. And I can @ mention the relevant folks. I'm afraid this thread will be too long for others to dig into :)

@ericsciple
Copy link
Collaborator

Also apologies on the delayed doc updates for include. Roll-out to all accounts went much slower than usual. I'll update the docs soon.

@eregon
Copy link

eregon commented Mar 31, 2020

This works nicely, thanks!

@MSP-Greg
Copy link
Author

@ericsciple

Sorry, it took me a little while to check this, but it is very helpful. Thank you.

In Ruby we've got a couple of edge cases that this makes really easy to implement, otherwise there's lots of repeated code.

I'll open an issue re the allow-failure/continue-on-error issue.

@MSP-Greg
Copy link
Author

MSP-Greg commented Nov 5, 2020

@ericsciple

In the process of changing workflows across several repos (remove -lastest, replace with numeric), I was reminded of the 'allow-failure' / continue-on-error.

See #2347 and below for some recent activity. In a couple ways, I made clear what I'd like to see.

But I'm crazy. Hope things are well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request external
Projects
None yet
Development

No branches or pull requests

3 participants