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 allow_failure #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add allow_failure #26

wants to merge 1 commit into from

Conversation

eteq
Copy link

@eteq eteq commented Nov 19, 2020

This PR makes a small change inspired by spacetelescope/jdaviz#369 - there I had a build I wanted to be an "Allowed failure" a la the Travis option. This PR does that using the continue-on-error github action option

@codecov
Copy link

codecov bot commented Nov 19, 2020

Codecov Report

Merging #26 (0425602) into main (4cb8db2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #26   +/-   ##
=======================================
  Coverage   96.72%   96.72%           
=======================================
  Files           4        4           
  Lines          61       61           
=======================================
  Hits           59       59           
  Misses          2        2           

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 4cb8db2...0425602. Read the comment docs.

@pllim
Copy link
Collaborator

pllim commented Nov 19, 2020

For this particular case, why not set fail-fast to false instead?

@pllim
Copy link
Collaborator

pllim commented Nov 19, 2020

There is no true "allow to fail" in Actions. That is, it will still show up as ❌ , it just won't stop other jobs from running.

@eteq
Copy link
Author

eteq commented Nov 19, 2020

For this particular case, why not set fail-fast to false instead?

In the case of spacetelescope/jdaviz#369 I was thinking there was one build that sometimes fails but I wanted all the other ones to have the "fail fast" behavior. (you pointed out the fix for that @pllim, so that particular build is now fixed, but the idea of having a way to indicate one of the builds should be allowed to fail is the intent here).

There is no true "allow to fail" in Actions. That is, it will still show up as x , it just won't stop other jobs from running.

We could change the wording because I agree it's not quite the same as in travis. But it looks to me like at least treating the whole build as ok, because if I go to https://github.com/eteq/jdaviz/actions and scroll down to #6, there's a green check next to it even though when you go into https://github.com/eteq/jdaviz/actions/runs/373116419 you see that it's got the red X.

@pllim
Copy link
Collaborator

pllim commented Nov 19, 2020

Ah, okay. Perhaps add some comments to explain your intent here, and maybe even comment them out for now? I am worried that people use this blindly and then resulting in running extra jobs unnecessarily when some of the jobs fail. That is, what they really need is "fail fast" but they are getting "run them all" from the template by default.

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.

2 participants