feat: provide on-pr workflow for all the basics checks that we need#89
feat: provide on-pr workflow for all the basics checks that we need#89AlexanderLanin merged 5 commits intomainfrom
Conversation
| bazel-format-check: | ||
| name: 🖌️ Run bazel //:format.check | ||
| needs: detect-repo-bazel-targets | ||
| if: needs.detect-repo-bazel-targets.outputs.has_format_check == 'true' |
There was a problem hiding this comment.
not sure if this is really helpful. I woul;d say it shall be manual workflow input defaulted to true so by default formating shall be enabled. NOw if you integrate it and do not have formatting in bazel job is just green without any indication
There was a problem hiding this comment.
I don't know whether formatting will be handled by bazel. Probably not. Maybe only in C++ / rust repos, maybe not, maybe not even there. The workflow is designed to be drop-in-able, rather than to enforce certain things.
| # so there is no reason to add any text where the icon is. | ||
| # --- | ||
| # Update: this name is actually not used anywhere! wtf? | ||
| name: 🔁 |
There was a problem hiding this comment.
It's common practice to use icons in job names, action names etc.
We already do that in most jobs in most infrastructure repos.
In this specific case I'll remove the name completely. It's not used anywhere, and there is no warning when it's not given. Will check the docs.
| bazel-copyright-check: | ||
| name: © Run bazel //:copyright.check | ||
| needs: detect-repo-bazel-targets | ||
| if: needs.detect-repo-bazel-targets.outputs.has_copyright_check == 'true' |
There was a problem hiding this comment.
Copyright is already moved to pre-commit AFAIK. So technically copyright.check is already legacy behavior.
| # Attention: this workflow must be called with following permissions: | ||
| # permissions: | ||
| # id-token: write | ||
| # contents: read |
There was a problem hiding this comment.
can this be checked in workflow ?
There was a problem hiding this comment.
There already is an error about missing permissions when you don't pass those. So a custom warning would be kind of pointless.
| common: | ||
| uses: ./.github/workflows/on-pr.yml |
There was a problem hiding this comment.
What happens if a job from on-pr.yml fails? I assume this one also fails.
I am asking because here I coded extra logic to check for failed jobs: https://github.com/eclipse-score/module_template/pull/57/changes#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR42-R62
Now I wonder if I could have achieved the same by using yet another workflow file.
There was a problem hiding this comment.
Interesting. Potentially yeah, the same can be achieved via another indirection. Let's try that after this PR is merged?!
AlexanderLanin
requested review from
MaximilianSoerenPollak,
PiotrKorkus,
dcalavrezo-qorix,
nradakovic and
opajonk
as code owners
github-project-automation
bot
moved this from Backlog
to Done
in INF - Infrastructure Community
3 participants
we have so many different things to run, that its super easy to forget something. and its super annoying to keep this consistent across repos. the new on-pr workflow fixes both issues, at the price of some skipped jobs showing up in PRs. That takes some getting used to, but it seems ok.
example in docs-as-code:
And of course the run of this PR here: https://github.com/eclipse-score/cicd-workflows/actions/runs/23822672623