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

Consolidate common behaviors between test and collateral jobs. #163

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jaraco
Copy link
Owner

@jaraco jaraco commented Feb 27, 2025

Closes #162

run: |
sudo apt update
sudo apt install -y libxml2-dev libxslt-dev
shell: bash
Copy link
Owner Author

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.

Copy link
Contributor

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@72d6defdf31803e9d8cbf0f97b10724dd72658a6 into 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.

Comment on lines +59 to +60
- name: Common
uses: ./.github/actions/common
Copy link
Owner Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

@jaraco
Copy link
Owner Author

jaraco commented Feb 27, 2025

I'm not looking forward to the merge toil this change is going to create.

@jaraco
Copy link
Owner Author

jaraco commented Feb 27, 2025

I'm going to sit on this one for a while. In the meantime, have a look at https://github.com/jaraco/jaraco.net/actions/runs/13577725111 for an example of it in action.

@jaraco
Copy link
Owner Author

jaraco commented Feb 27, 2025

I had another idea to use YAML anchors. I'll compare that approach before merging here.

@jaraco
Copy link
Owner Author

jaraco commented Feb 28, 2025

I've started work in feature/anchor branch, but it isn't yet supported.

@bswck
Copy link
Contributor

bswck commented Mar 1, 2025

Fun fact: GitLab CI does support YAML anchors.

@jaraco
Copy link
Owner Author

jaraco commented Mar 1, 2025

Fun fact: GitLab CI does support YAML anchors.

Not according to this run

image

What makes you think they're supported?

@bswck
Copy link
Contributor

bswck commented Mar 1, 2025

I said GitLab CI, not GitHub 😄

@webknjaz
Copy link
Contributor

webknjaz commented Mar 9, 2025

Fun fact: GitLab CI does support YAML anchors.

FWIW, they are considered controversial for a number of reasons. And while I used to use them heavily in the Travis CI times, I was often seeing confusion when other humans would attempt parsing them. It can be a UX disaster.

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

Successfully merging this pull request may close these issues.

tests and collateral share some behavior implicitly
3 participants