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 #375

Merged
merged 1 commit into from
Nov 24, 2022

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.

It is now in use in aiohttp (and other aio-libs projects), CherryPy, attrs, coveragepy, conda some of the Ansible repositories, pip-tools, pydantic, spaceship-prompt, Towncrier, all of the jaraco's projects (like setuptools, importlib_metadata), some PyCQA, PyCA, PyPA and pytest projects, a few AWS Labs projects.

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

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.

It is now in use in aiohttp (and other aio-libs projects), CherryPy,
attrs, coveragepy, conda some of the Ansible repositories, pip-tools,
pydantic, spaceship-prompt, Towncrier, all of the jaraco's projects
(like `setuptools`, `importlib_metadata`), some PyCQA, PyCA, PyPA and
pytest projects, a few AWS Labs projects.

The story behind this is explained in more detail at
https://github.com/marketplace/actions/alls-green#why.
@webknjaz webknjaz mentioned this pull request Nov 21, 2022
@jorisroovers
Copy link
Owner

So we currently don't have any branch protection rules setup for gitlint. In fact, I still fairly regularly push directly to main (after running tests locally and sometimes using force push to fix issues on main). I realize I should stop doing that 😅 I never merge other branches into main directly though, that I always do via a PR.

Given that only @sigmavirus24 and I have commit permission on this repo this hasn't really been an issue.

My understanding is that this PR is only useful in case branch permissions have been setup. I've used branch protection rules before but I'm interested to hear your advise on some best-practices wrt branch protection rules. Thanks!

@webknjaz
Copy link
Contributor Author

My understanding is that this PR is only useful in case branch permissions have been setup. I've used branch protection rules before but I'm interested to hear your advise on some best-practices wrt branch protection rules. Thanks!

There are other useful bits to this:

  • in the graph view at https://github.com/jorisroovers/gitlint/actions/runs/3516847917, there is a distinct check that represents the overall outcome of the workflow, it's easier to spot, less mental overhead
  • the PR checks lists, it'd be marked as required — it's a useful signal to the contributors who may not know whether all the jobs are expected to fail or of some are unstable and are fine to fail — this also reduces the mental load
  • it is useful when you enable the auto-merge feature — having a branch protection check would enable you to click auto-merge on PRs and switch to other things knowing that it'll be merged when and if the CI passes, no need to baby-sit the CI, waiting for the change to click Merge only after that

Regarding the push privileges, you can have a branch protection rule for the sake of setting the expectations but still allow the administrators to push to the branch, bypassing the mandatory checks. This means that both direct pushes and force-merges will still work.

@jorisroovers
Copy link
Owner

Ok. Seems like it has mostly mental load benefits for gitlint at this point, but allows for more powerful automation when branch protection and auto-merge features would be used - something you've now got me thinking about :-)

Merging this in - thanks for submitting and educating me along the way!

@jorisroovers jorisroovers merged commit 11635b9 into jorisroovers:main Nov 24, 2022
jorisroovers added a commit that referenced this pull request Mar 7, 2023
This release was primarily focussed on modernizing gitlint's build and test
tooling (details: #378).

General

    Python 3.6 no longer supported (EOL since 2021-12-23) (#379)
    This is the last release to support the sh library (used under-the-hood to
    execute git commands) by setting GITLINT_USE_SH_LIB=1. This is already
    disabled by default since v0.18.0.

Features

    Allow for a single commit in the --commits cmd-line param (#412)
    Gitlint now separates FILE_ENCODING (always UTF-8) from TERMINAL_ENCODING
    (terminal dependent), this should improve issues with unicode. Use
    gitlint --debug to inspect these values. (#424)

Bugfixes

    ignore-by-author-name crashes without --staged (#445)
    Various documentation fixes (#401, #433) - Thanks scop

Development

    Adopted hatch for project management (#384). This significantly improves
    the developer workflow, please read the updated CONTRIBUTING page.
    Adopted ruff for linting, replacing pylint (#404)
    Gitlint now publishes dev builds on every commit to main (#429)
    Gitlint now publishes a latest_dev docker image on every commit to
    main (#451) (#452)
    Dependencies updated
    Many improvements to the CI/CD worfklows
    Improve unit test coverage (#453)
    Integration test fixes on windows (#392, #397)
    Devcontainer improvements (#428)
    Removal of Dockerfile.dev (#390)
    Fix most integration tests on Windows
    Fix Windows unit tests (#383)
    Introduce a gate/check GHA job (#375)

Full Release details in CHANGELOG.md.
jorisroovers added a commit that referenced this pull request Mar 7, 2023
This release was primarily focussed on modernizing gitlint's build and test
tooling (details: #378).

General

    Python 3.6 no longer supported (EOL since 2021-12-23) (#379)
    This is the last release to support the sh library (used under-the-hood to
    execute git commands) by setting GITLINT_USE_SH_LIB=1. This is already
    disabled by default since v0.18.0.

Features

    Allow for a single commit in the --commits cmd-line param (#412)
    Gitlint now separates FILE_ENCODING (always UTF-8) from TERMINAL_ENCODING
    (terminal dependent), this should improve issues with unicode. Use
    gitlint --debug to inspect these values. (#424)

Bugfixes

    ignore-by-author-name crashes without --staged (#445)
    Various documentation fixes (#401, #433) - Thanks scop

Development

    Adopted hatch for project management (#384). This significantly improves
    the developer workflow, please read the updated CONTRIBUTING page.
    Adopted ruff for linting, replacing pylint (#404)
    Gitlint now publishes dev builds on every commit to main (#429)
    Gitlint now publishes a latest_dev docker image on every commit to
    main (#451) (#452)
    Dependencies updated
    Many improvements to the CI/CD worfklows
    Improve unit test coverage (#453)
    Integration test fixes on windows (#392, #397)
    Devcontainer improvements (#428)
    Removal of Dockerfile.dev (#390)
    Fix most integration tests on Windows
    Fix Windows unit tests (#383)
    Introduce a gate/check GHA job (#375)

Full Release details in CHANGELOG.md.
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.

2 participants