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 a validate job to the workflows #33

Open
briantist opened this issue Apr 10, 2022 · 1 comment
Open

Add a validate job to the workflows #33

briantist opened this issue Apr 10, 2022 · 1 comment

Comments

@briantist
Copy link
Collaborator

With #27 , we have some control over how strict the build is.

The way I see it, there should be a build that has the strictest settings, for validation, while builds for publishing (especially in PRs) should use the most lenient options, to give us the best chance of a publishable build (even if some pages have errors), because that's more useful for checking rendered output and comparing changes.

By using separate jobs, we can get published docs whenever possible, while still having the validate job "fail" the CI until all errors/warnings are fixed.

Currently using this pattern in community.hashi_vault and lowlydba.sqlserver, but it could use support here to avoid having to repeat some things.

In the push workflows, we can achieve this with the push reusable workflow already, with caveats:

  • we can't disable the artifact upload, so we set the name to _unused
    • the underlying action can already disable artifact upload, we should expose that to the push workflow

In PR, the only reason we can't use the push reusable workflow is because it has no way to specify the ref, so:

  • we kind of repeat what the push workflow does but use the merge commit for checkout
    • the push workflow could take a ref input
  • we skip on PR close events (no need to validate there)
  • should we add a validate reusable workflow?
    • if so, we cannot use it in the other reusable workflows (can't nest reusables), so it will be some duplication
    • but it does provide a convenient use for users who want to the run the validations separately
  • inclusion of a validate job in PR and Push workflows
    • good (opinionated) default imo
    • provide option to skip it? easy to do, some collections maybe don't want to fail CI on this
    • if optional job is used in needs: it complicates the dependent jobs a little if you still want them to run; need to use if: always() && needs.validate.result [is skipped or successful] type of conditional
    • validate is not needed in needs: for PRs, but probably would be for push workflows that also publish, if that validation should prevent publish; in theory the push workflow can just be invoked most strictly all the time, removing the need for a separate validation workflow, but this won't work for pre-init'd use cases, unless strictness can be controlled on build as well as init.

Last point leads to the idea of:

  • anstibull-docs sphinx-init generated build.sh could be updated to take arguments, allowing for some override of options chosen at init time
@briantist
Copy link
Collaborator Author

#45 makes this possible. At the moment, it's still on the choice of the consumer to do it. We can add it so samples.

On the fence still whether we should build it into the workflows or not. Or have more workflow (some with some without).

I think better to keep this open and revisit once I can propose an actual change to build.sh.

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

No branches or pull requests

1 participant