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

Add a skip ci feature to the CI workflow #5591

Closed

Conversation

eslavich
Copy link
Collaborator

@eslavich eslavich commented Jan 5, 2021

This replicates the Travis feature where [skip ci] or [ci skip] in the commit message causes tests to be skipped.

@eslavich eslavich force-pushed the eslavich-add-ci-skip-feature branch from 5fa0f1d to f232e1a Compare January 5, 2021 19:15
@codecov
Copy link

codecov bot commented Jan 5, 2021

Codecov Report

Merging #5591 (e4166ac) into master (93b8d5d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5591   +/-   ##
=======================================
  Coverage   74.39%   74.39%           
=======================================
  Files         416      416           
  Lines       38088    38088           
  Branches     5637     5637           
=======================================
  Hits        28336    28336           
  Misses       9752     9752           
Flag Coverage Δ *Carryforward flag
nightly 75.93% <ø> (ø) Carriedforward from 93b8d5d
unit 54.56% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93b8d5d...f851dd4. Read the comment docs.

@pllim
Copy link
Contributor

pllim commented Jan 6, 2021

Duplicate functionality as #5577 but different approach.

@pllim
Copy link
Contributor

pllim commented Jan 6, 2021

p.s. Is the addition of a file named foo intentional?

@eslavich
Copy link
Collaborator Author

eslavich commented Jan 6, 2021

p.s. Is the addition of a file named foo intentional?

Yea, that was just the change I made to show the behavior when [skip ci] is present. I'll remove that commit if we end up deciding to merge this.

@eslavich
Copy link
Collaborator Author

eslavich commented Jan 7, 2021

I attempted to make this version a reusable action, but evidently we're not permitted to nest an action (like checkout) within another action. I think @pllim's javascript action is the way to go, though I'm in favor of setting an output and checking it with an if: so that we can avoid a red X failure when the CI is skipped.

@pllim
Copy link
Contributor

pllim commented Jan 7, 2021

Yeah, composite action isn't a thing yet though it is widely requested. You can wait, but I am not sure for how long.

I'm in favor of setting an output and checking it

Looks like there is an option to set output in https://github.com/actions/toolkit/tree/main/packages/core . But I have to think about this. Maybe I'll make it an option whether to fail or just set the output, but that also complicates the documentation and logic.

@pllim
Copy link
Contributor

pllim commented Jan 7, 2021

For now though, if you can endure a red X, my action already works, as illustrated in the other PR, in case you need it immediately.

@jdavies-st
Copy link
Collaborator

Yeah, I don't think we want to endure red Xs for skipped CI. For jwst, the check should pass if CI is skipped.

Same is true for allowed failures, though that's a separate issue.

The way all this worked on Travis CI was very useful and we'd like to see that identical functionality in Github actions as well.

I would very much be in favor of the best of both these PRs getting put together.

@pllim
Copy link
Contributor

pllim commented Jan 7, 2021

That makes sense. I am looking into it.

@pllim
Copy link
Contributor

pllim commented Jan 7, 2021

I updated #5577 with your requested feature. Took me a while to get the output syntax right, but it seems to work now.

@eslavich
Copy link
Collaborator Author

eslavich commented Jan 8, 2021

Ok thanks a lot! Closing this one.

@eslavich eslavich closed this Jan 8, 2021
@pllim
Copy link
Contributor

pllim commented Jan 11, 2021

Here is the issue about composite action actions/runner#646

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants