coveragepy
coveragepy copied to clipboard
nedbat
🎨🧪 Wire linters into the main CI workflow
This patch makes the quality workflow definition into reusable. It doesn't need to track the same triggers and conditions as the main one. Additionally, the alls-green check can now take into account the outcome of linting making it possible to only have one job in the branch protection.
I'm missing what problem this solves? Conceptually, tests and linting are separate to me, so deserve separate workflows.
Yes, I understand that the linters may be conceptually different. Though, they still remain in a separate workflow definition file but show up on the same screen with other jobs. As in this graph: https://github.com/nedbat/coveragepy/actions/runs/5526202109?pr=1652. The checks still show up in the same widget at the bottom of PRs too.
But wiring that into the unified check job helps prevent forgetting to change branch protection, which also tends to be set by linters. And being able to do so also opens up some possibilities, like file-dependent job runs that can still make use of branch protection (this is the problem I was solving for CPython recently, for example).
I see that https://github.com/nedbat/coveragepy/commit/42508990e08865ba93e8a893d36061351e553a63 dropped mypy from the branch protection and maybe re-added and re-removed it. But it's hard to say since these changes aren't traceable. With everything wired in, it'd be possible to see what jobs and under which condition influence the final branch protection check.
Of course, it's your call, but I've been rolling out this approach of modular workflow definitions in several places recently, and it seems to work very well.
I guess I don't understand the reason for making it reusable, just to use it from one workflow? Why not embed it directly in the workflow? I must be missing something.
just to use it from one workflow?
Yes, a single workflow runtime is viewable as a single entity on the UI.
Why not embed it directly in the workflow?
It's possible, but it's usually nice to have separate job definitions in dedicated files. Having them in their own files lets us avoid having an oversized main workflow definition that ties them together. At the same time, this allows to consolidate the triggers in the main workflow and not have to keep them in sync through many files. Think of Python — we put things into separate modules to structure and group stuff. But it is also a layer separation mechanism. Low-level implementation may be separate from the top-level control flow.
https://learn.scientific-python.org/development/guides/gha-basic/#reusable-workflows
I must be missing something.
I think the idea here is that yes, it's nice to have modularized definitions for semantically different jobs. And at the same time, it's hard to maintain huge YAML files. So this concept is a good middle ground in my opinion. FWIW, after I used it in a few repos, I liked it a lot and plan to use in my other projects.