ci: run type, template and migration checks - #234
Closed
JediBrooker wants to merge 1 commit into
Closed
JediBrooker wants to merge 1 commit into
JediBrooker wants to merge 1 commit into
Conversation
The workflow was named "Test types" but only ran `just test_format`, so `just types` and `just test_jinja` existed in the justfile without anything calling them. A type error or a template that fails to render could reach main. Adds both to the workflow, plus a new `check_migrations` recipe that runs `alembic upgrade heads` followed by `alembic check`, which CONTRIBUTING already asks contributors to run by hand. Also fixes when the workflow runs: - adds a `pull_request` trigger. build.yaml has one, this workflow did not, so none of these checks ran on a pull request at all. - adds `**.jinja`, `justfile` and `uv.lock` to the paths. Template changes did not trigger the workflow before, despite being what `test_format` mainly checks.
Owner
|
All the necessary checks are already performed in the build(/test) pipeline: https://github.com/markbeep/AudioBookRequest/blob/main/.github/workflows/build.yaml This change is not necessary. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The workflow is named
Test typesbut only runsjust test_format.just typesandjust test_jinjaare both defined in the justfile with nothing calling them, so a type error or a template that fails to render can reachmain.This wires up what's already there and adds the migration check CONTRIBUTING asks contributors to run by hand.
Changes
Runs the existing recipes
just types—basedpyright, which the justfile already definesjust test_jinja—app/util/test_jinjax.py, likewiseNew
check_migrationsrecipeCONTRIBUTING already says "Ensure there are no missing migrations:
uv run alembic check", so this just stops it being a thing people have to remember. NeedsABR_APP__CONFIG_DIRset, which the workflow does;alembic/env.pyalready creates the directory, so no setup step is needed.Fixes when the workflow runs
pull_requesttrigger.build.yamlhas one and this workflow didn't, so none of these checks ran on a pull request at all — only on pushes to branches in this repo. Same path list as the push trigger, matching howbuild.yamldoes it.**.jinjato the paths. Template changes didn't trigger the workflow before, despite templates being the main thingtest_formatchecks.justfileanduv.lock, since changes to either can break the checks.Renamed the job to
Format, types and migrationsso the name matches what it does.Checked
Ran all four steps against a clean checkout of
main:just test_formatjust typesjust test_jinjajust check_migrationsSo this shouldn't turn anything red on merge — it's all currently passing, just not being run.
Written with AI assistance.