Skip to content
This repository was archived by the owner on Dec 5, 2022. It is now read-only.

Return success code when nothing to commit#9

Merged
maxheld83 merged 5 commits intomaxheld83:masterfrom
leny:feature/neutral-exit
Feb 20, 2019
Merged

Return success code when nothing to commit#9
maxheld83 merged 5 commits intomaxheld83:masterfrom
leny:feature/neutral-exit

Conversation

@leny
Copy link
Copy Markdown
Contributor

@leny leny commented Feb 20, 2019

Here's an implementation for the issue addressed in issue #7

The process return a neutral when there's nothing to commit, instead of an error.

@leny
Copy link
Copy Markdown
Contributor Author

leny commented Feb 20, 2019

(sorry for the revert, I erronously included the commit from PR #8 - the revert is to fix that)

@maxheld83
Copy link
Copy Markdown
Owner

maxheld83 commented Feb 20, 2019

Uh I'm a little confused about this, so correct me if I have this all wrong @leny.

I see two separate issues here:

1. The current action appears to error out when there is nothing to commit.

This does not, on the face of it, make much sense.
I can think of any number of scenarios where, for some commits to the source do not cause a diff in the build asset dir/.
Erroring it clearly isn't the right response here, because there's nothing wrong per se.
I also tend to think that 78 status might not be quite right, because as per the GitHub actions spec:

When an action returns this exit status, GitHub terminates all concurrently running actions and prevents any future actions from starting.

This, again, in some scenarios isn't what users might want or expect.

Also, I think this problem should only occur in the first place for deploys from . where there already (qua github actions provisioning of the workspace) is a git repo. In any subdir/, there won't be a git repo, and so there'd by definition always be something to commit.
Thus just as an aside.

2. If deploying from ., this might cause an "infinite loop" of sorts (if 1 were fixed)..
That's also bad.
I wonder though, whether the correct response to this wouldn't be to:

  1. Commit sources to one branch, and deploy from some other branch (gh-pages).
  2. Simply add a filter action before the deploy, and have the workflow continue only for !gh-pages or even just master, or whatever the plan may be.

@maxheld83
Copy link
Copy Markdown
Owner

Sorry this got more complicated than expected.

So my (preliminary) suggestion would be to just exit 0 instead of exit 78 for empty commits and rely on filter actions to avoid infinite loops.

@leny Does that make sense? Does that meet user expectations and keep things simple to reason about?

Set me straight if I got this wrong.

maxheld83 added a commit that referenced this pull request Feb 20, 2019
added warning as per problem faced by @leny.
@leny
Copy link
Copy Markdown
Contributor Author

leny commented Feb 20, 2019

The more I think of it, the more I think the exit 0 is what makes more sense, and the other cases should be addressed with a filter action, as you suggest.

@maxheld83
Copy link
Copy Markdown
Owner

perfect, will merge, test and ship this then.
Thanks for the quick turnaround and great contributions @leny!

@maxheld83 maxheld83 merged commit ac211ab into maxheld83:master Feb 20, 2019
@maxheld83 maxheld83 changed the title Return neutral code when nothing to commit Return success code when nothing to commit Feb 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants