nextstrain/auspice

CI: Should warnings should cause the lint job to fail?

joverlee521 opened this issue · 2 comments

Originally proposed by @victorlin in #1837 (comment)

Proposal to prevent this in the future: warnings should cause the lint job to fail (how and why)

If the lint job fails due to warnings, the current publish job will not run:

publish:
if: ${{ github.ref == 'refs/heads/release' }}
needs: [build, unit-test, smoke-test, lint, check-lockfile]

This would force us to resolve all lint warnings before releasing

This would force us to resolve all lint warnings before releasing

Seems fine to me. I wouldn't expect this to happen often (if at all). Changes that make their way into master through a proper PR will surface warnings on the PR itself which should be resolved before merging. The only time when this might happen is if a commit is pushed directly to master. In that case I think it's reasonable to fix up before release. If we really wanted to for whatever reason, we could temporarily tweak the rule to remove the lint dependency from publish.