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

🧪 Introduce a gate/check GHA job #635

Merged
merged 3 commits into from
Jun 5, 2023

Conversation

webknjaz
Copy link
Contributor

This adds a GHA job that reliably determines if all the required dependencies have succeeded or not.

It also allows to reduce the list of required branch protection CI statuses to just one — check. This reduces the maintenance burden by a lot and have been battle-tested across a small bunch of projects in its action form and in-house implementations of other people.

This action is now in use in aiohttp (and other aio-libs projects), CherryPy, conda, coveragepy, Open edX, Towncrier some of the Ansible repositories, pip-tools, all of the jaraco's projects (like setuptools, importlib_metadata), some of hynek's projects (like attrs, structlog), some PyCQA, PyCA, PyPA and pytest projects, a few AWS Labs projects. Admittedly, I maintain a few of these but it seems to address some of the pain folks have:
jaraco/skeleton#55 (comment).

I figured, this might be useful for MyST too, which is why I'm submitting this patch.

The story behind this is explained in more detail at https://github.com/marketplace/actions/alls-green#why.

@@ -36,6 +36,12 @@ jobs:
python-version: "3.8"
sphinx: ">=4,<5"

continue-on-error: >-
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisjsewell this is here to match the currently set up branch protection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this as I think its not necessary to ignore any

@@ -103,10 +109,29 @@ jobs:
- name: Run docutils CLI
run: echo "test" | myst-docutils-html

# https://github.com/marketplace/actions/alls-green#why
check: # This job does nothing and is only used for the branch protection
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what should eventually replace all the individual branch protection checks that belong to this workflow.

uses: re-actors/alls-green@release/v1
with:
jobs: ${{ toJSON(needs) }}

publish:

name: Publish myst-parser to PyPi
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An unrelated comment/suggestion for the future: this could use an environment setting and secrets could be scoped to it instead of being accessible globally.

This adds a GHA job that reliably determines if all the required
dependencies have succeeded or not.

It also allows to reduce the list of required branch protection CI
statuses to just one — `check`. This reduces the maintenance burden
by a lot and have been battle-tested across a small bunch of projects
in its action form and in-house implementations of other people.

This action is now in use in aiohttp (and other aio-libs projects),
CherryPy, conda, coveragepy, Open edX, Towncrier some of the Ansible
repositories, pip-tools, all of the jaraco's projects (like
`setuptools`, `importlib_metadata`), some of hynek's projects (like
`attrs`, `structlog`), some PyCQA, PyCA, PyPA and pytest projects, a
few AWS Labs projects. Admittedly, I maintain a few of these but it
seems to address some of the pain folks have:
jaraco/skeleton#55 (comment).

I figured, this might be useful for MyST too, which is why I'm
submitting this patch.

The story behind this is explained in more detail at
https://github.com/marketplace/actions/alls-green#why.
@chrisjsewell chrisjsewell changed the title Introduce a gate/check GHA job 🧪 Introduce a gate/check GHA job Jun 5, 2023
@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (41582e2) 90.00% compared to head (c6cae3f) 90.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #635   +/-   ##
=======================================
  Coverage   90.00%   90.00%           
=======================================
  Files          23       23           
  Lines        2970     2970           
=======================================
  Hits         2673     2673           
  Misses        297      297           
Flag Coverage Δ
pytests 90.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chrisjsewell chrisjsewell merged commit 19b00d2 into executablebooks:master Jun 5, 2023
21 checks passed
@welcome
Copy link

welcome bot commented Jun 5, 2023

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@chrisjsewell
Copy link
Member

Cheers!

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.

None yet

2 participants