10up/action-wordpress-plugin-asset-update

Change neutral exit for no changes found to success

Closed this issue · 3 comments

helen commented

Describe the bug

In the previous HCL-based Actions beta version, you could mark an exit as neutral by using exit 78. This no longer works for good reason, per https://twitter.com/ethomson/status/1163899559279497217. In practice this means that a declined readme/asset update due to no changes being found gets marked as errored where it isn't actually an error (declining to update due to other changes being found is properly an error). This issue is to acknowledge this IMO unexpected behavior and hold for whenever GitHub gets around to implementing a new neutral exit method in this YAML-based beta version.

Steps to Reproduce

  1. Make changes to a file that isn't deployed to .org, like an Actions workflow file or a test.
  2. Push to master (or whatever branch you have marked for the readme/asset deploy Action).
  3. Go to Actions.
  4. See that the Action failed with the message [error]Docker run failed with exit code 78

Expected behavior
I would expect the Action to be marked as neutral, not failed.

helen commented

Thinking about it more, I am open to more discussion on whether other changes being found should be marked as an error, it does feel kind of funny to have something like a new release be marked as half-success half-error because it successfully deploys the new tag to .org but doesn't do the readme/asset updater because of other changes. Maybe the only error state is if SVN fails or some other bug in the script.

2ndkauboy commented

I think the main issue is the fact, that both actions start parallel. This will cause the readme/asstes action to "fail". I have not gotten into GitHub actions too much, but is there a way to chain them? I use GitLab-CI a lot and something like this is possible there.

helen commented

@2ndkauboy The new triggers actually no longer make them run on the same pushes; deploy runs on pushes of a new tag, this runs on pushes to master but not when they're tags. That said, it's really starting to bug me that this fails when it's not truly a failure, so I think I'm going to mark the case of no changes being found as a success instead. The case of "more stuff has been updated" failing is somewhat annoying when your merge is in preparation for a new release, but for the moment that still seems more correct since that might be inadvertent.