-
Notifications
You must be signed in to change notification settings - Fork 38
Consolidate common behaviors between test and collateral jobs. #163
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| name: "Common Steps" | ||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Install build dependencies | ||
| # Install dependencies for building packages on pre-release Pythons | ||
| # jaraco/skeleton#161 | ||
| if: matrix.python == '3.14' && matrix.platform == 'ubuntu-latest' | ||
| run: | | ||
| sudo apt update | ||
| sudo apt install -y libxml2-dev libxslt-dev | ||
| shell: bash | ||
| - name: Setup Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: ${{ matrix.python }} | ||
| allow-prereleases: true | ||
| - name: Install tox | ||
| run: python -m pip install tox | ||
| shell: bash | ||
| - name: Run | ||
| run: tox | ||
| shell: bash | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,23 +55,9 @@ jobs: | |
| runs-on: ${{ matrix.platform }} | ||
| continue-on-error: ${{ matrix.python == '3.14' }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Install build dependencies | ||
| # Install dependencies for building packages on pre-release Pythons | ||
| # jaraco/skeleton#161 | ||
| if: matrix.python == '3.14' && matrix.platform == 'ubuntu-latest' | ||
| run: | | ||
| sudo apt update | ||
| sudo apt install -y libxml2-dev libxslt-dev | ||
| - name: Setup Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: ${{ matrix.python }} | ||
| allow-prereleases: true | ||
| - name: Install tox | ||
| run: python -m pip install tox | ||
| - name: Run | ||
| run: tox | ||
| - uses: actions/checkout@v4 | ||
| - name: Common | ||
| uses: ./.github/actions/common | ||
|
Comment on lines
+59
to
+60
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other degradation that occurs with composite workflows is all the inner steps get folded into this "Common" step.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...which is why I prefer reusable workflows to actions. Actions are there to encapsulate a single visual step, while workflows would show all. |
||
|
|
||
| collateral: | ||
| strategy: | ||
|
|
@@ -80,19 +66,20 @@ jobs: | |
| job: | ||
| - diffcov | ||
| - docs | ||
| runs-on: ubuntu-latest | ||
| python: | ||
| - "3.x" | ||
| platform: | ||
| - ubuntu-latest | ||
| env: | ||
| TOX_ENV: ${{ matrix.job }} | ||
| runs-on: ${{ matrix.platform }} | ||
| continue-on-error: ${{ matrix.python == '3.14' }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Setup Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: 3.x | ||
| - name: Install tox | ||
| run: python -m pip install tox | ||
| - name: Eval ${{ matrix.job }} | ||
| run: tox -e ${{ matrix.job }} | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Common | ||
| uses: ./.github/actions/common | ||
|
|
||
| check: # This job does nothing and is only used for the branch protection | ||
| if: always() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't love that moving to a composite workflow requires bash be specified when it wasn't before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can change to reusable workflows instead of composite actions to combat this, FYI. (You seem to confuse the terminology here?)
I want to share something. I'm not advertising this very much, but I'm working on something in this space, too.
It's a functional PoC that is not in its own repository just yet, and I don't consider it fully ready for prime time. But you can play with https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/72d6def/.github/workflows/reusable-tox.yml by adding something like
uses: sphinx-contrib/sphinxcontrib-towncrier/.github/workflows/reusable-tox.yml@72d6defdf31803e9d8cbf0f97b10724dd72658a6into one of your experimental branches. Just pass all the required inputs into it.It will end up in https://github.com/stdlib-workflows (or perhaps https://github.com/tox-dev/workflow) a bit later: for now, I'm copying it around across a few projects while I'm testing how well this thing covers various corner cases.
It's designed to work with matrixes defined in the top-level calling workflow, just like you have here.
To enable project-specific customization, I'm currently trying to work out what hooks it needs.
Here's one: https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/72d6def/.github/stdlib-workflows/reusable-tox/actions/post-src-checkout/action.yml. It is invoked if a calling project has an action with a specific name in a specific location.
So now I'm looking to check what parts of the reusable workflow could be customizable. Specifically, I want to instrument it with more hooks (there's one job that doesn't entirely fit the pattern — https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/72d6def/.github/workflows/ci-cd.yml#L311-L470 — so I'm looking into what it needs). I'll try to make those hooks overrideable per-toxenv. And it looks like it'll be useful to pass some sort of context from the calling workflow for access from within the hooks and expose the outputs of each hook through the reusable workflow, so the top-level one could do something about it.