jenkins-infra / jenkins-maven-cd-action

1 stars 7 forks source link

Separate action or step for interesting category? #5

Closed timja closed 2 years ago

timja commented 3 years ago

I don't think I'm a fan of all the failed actions that happen when I merge, After I merge I get a number of failed checks including the release one if the release isn't interesting...

@garethjevans is doing some experimenting here with other options: https://github.com/jenkinsci/tekton-client-plugin/blob/master/.github/workflows/cd.yaml

I think some of this should be done via outputs and if blocks

i.e. a new action that checks if a release is interesting with an output that can be used to validate the release?

and verify status could return an output if the check is Jenkins and it's success...

WDYT @garethjevans and @jglick ?

garethjevans commented 3 years ago

Agreed, I like the idea of separating out the interesting check from the rest of the release and running the release step based on the output of the previous step.

This seems to be working nicely for us on the tekton-client-plugin.

I'd also recommend using the pr-labeler to get the correct labels automatically added to the PR for the release. e.g. https://github.com/jenkinsci/tekton-client-plugin/blob/master/.github/workflows/pr-labeler.yaml & https://github.com/jenkinsci/tekton-client-plugin/blob/master/.github/labeler.yml

jglick commented 3 years ago

Let us set aside PR labelling as a separate system you could do with a separate action with its own flow on open PRs. Most Jenkins PRs do not use this K8s-style naming convention. We normally apply labels manually.

As to failing checks: the ::set-output system can certainly work. As I think I discussed in the JEP, the downside is that it requires more config in each repo, and more knowledge of Actions to understand and debug. There is no standard way to manage Action templates at scale, so the longer the YAML the more work to keep everyone up to date. Not really sure what to do about this.

timja commented 3 years ago

Let us set aside PR labelling as a separate system you could do with a separate action with its own flow on open PRs. Most Jenkins PRs do not use this K8s-style naming convention. We normally apply labels manually.

👍 (aka 'semantic commits')

As to failing checks: the ::set-output system can certainly work. As I think I discussed in the JEP, the downside is that it requires more config in each repo, and more knowledge of Actions to understand and debug. There is no standard way to manage Action templates at scale, so the longer the YAML the more work to keep everyone up to date. Not really sure what to do about this.

I wouldn't expect them to change much, GitHub does have action templates on their roadmap but they aren't available yet.

I found this while searching which could help: https://github.com/varunsridharan/action-github-workflow-sync

Also looks like we can setup an action template to make it easier to onboard people: https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization

For now I assume we would just publish with a semver major bump when changing to a set-output rather than a exit code of non 0

jglick commented 3 years ago

GitHub does have action templates on their roadmap

https://github.com/github/roadmap/issues/98 I guess. More helpful discussion in https://github.com/actions/runner/issues/646.

I found this while searching which could help

This one is not much good I am afraid—requires a PAT.

set up an action template to make it easier to onboard people

This would make cd.yaml a bit easier to set up. OTOH it means that activating JEP-229 is no longer done via an atomic PR that others can look at and copy (adjusting metadata files and Release Drafter). And of course it does nothing to help us propagate subsequent tuning or make sure repositories are not drifting into snowflake configs.

we would just publish with a semver major bump when changing to a set-output

Better to opt in with a flag, or just create a separate action.

See also: https://github.com/jenkinsci/jep/blob/master/jep/229/README.adoc#single-action-for-release-ci-check-and-actual-release

timja commented 3 years ago

Resolved in https://github.com/jenkinsci/.github/pull/48, just needs an update to docs in https://github.com/jenkinsci/incrementals-tools to close this out

jglick commented 3 years ago

just needs an update to docs in https://github.com/jenkinsci/incrementals-tools to close this out

Still outstanding?

timja commented 3 years ago

yeah sorry haven't had a chance yet 😢

jglick commented 2 years ago

I do not see anything to be done in https://www.jenkins.io/doc/developer/publishing/releasing-cd/.