lychee-action icon indicating copy to clipboard operation
lychee-action copied to clipboard

Fail pipeline on error by default (fixes #71)

Open mre opened this issue 3 years ago • 1 comments

Previously, a pipeline would fail if fail was set to true. In the beginning I wasn't sure if this should be the default, so this is an opt-in. Feedback from users showed that not failing the pipeline on broken links is somewhat unexpected (compare #71). Therefore I'd like to change this and make failing the default.

Since this a potentially breaking change, I consider releasing this as part of v2.0.0.

@dkarlovi fyi

mre avatar Mar 01 '22 22:03 mre

From https://github.com/lycheeverse/lychee-action/issues/71#issuecomment-1022325442

  1. the thought process was that lychee was used as a step within a bigger "linting" pipeline and we didn't want to fail the entire pipeline because of a broken link.

    The workflow maintainer should use continue-on-error: true for lychee-action in this case. They can still check the status of lychee-action specifically with steps.lychee.outcome.

    Neither exit_status output, nor fail input for lychee-action seem necessary for this action. The purpose they serve is already supported with official workflow syntax that applies for all steps generically.

  2. Would like to see some strong arguments in favor of failing the pipeline

    Steps can run only when a failure occurs, such as creating an issue report.

    That does not require the pipeline/job to fail however. continue-on-error: true will allow lychee-action to always be "successful" (steps.lychee.conclusion), but steps that are interested can still know if the step actually failed (steps.lychee.outcome).

If you'd like to know more, I have gone into more detail (with references to official GH action docs) in a related PR discussion.


I think it would be better to drop both outputs.exit_status + inputs.fail for 2.0, and raise awareness via documentation of their equivalents:

  • steps.lychee.continue-on-error: true:
    • Don't fail a job early when the step fails (sets: steps.lychee.conclusion = 'success').
    • Step retains failure status (steps.lychee.outcome = 'failure').
  • steps.other-step.if: ${{ failure() && steps.lychee.outcome == 'failure' }}:
    • steps.other-step is only run when steps.lychee step fails.

polarathene avatar Jul 03 '22 05:07 polarathene

I think it would be better to drop both outputs.exit_status + inputs.fail for 2.0, and raise awareness via documentation of their equivalents.

I see your point, but I'm not fully convinced. While the methods you've described are correct, they may not be straightforward for everyone. Not all users might take the time to read through the documentation to find that specific note. Having a clear argument in lychee-action makes it easier to find and use.

Also, we already have a fail parameter, which makes it easy to adjust. Users can just set fail: false if the update causes any problems. We'll make sure to point this out in the release notes.

I think it's important to go ahead with this update, since it has been pending for a while. Let's make this small improvement now. We can always come back to this topic in version 3, making any adjustments easier then.

mre avatar Apr 25 '24 12:04 mre