ci: standardise pre-commit, lint and release tooling - #1504
Merged
Conversation
Add a pre-push hook that runs pre-commit over the whole repository, so a push is rejected when any file is unclean. The commit hook keeps checking staged files only. Replace the divergent per-app setups with one shared toolchain: ruff for lint and format, the self-contained frappe-semgrep hook, and commitlint for commit messages. Anchor the exclude regex so .github/ is no longer skipped by an unanchored .git pattern. Add the pre-commit and semantic-commits workflows, and scripts/setup-git-hooks.sh for a one-command developer bootstrap. Remove ci.yml. Building a bench and migrating a throwaway site cost five to eight minutes per pull request and went red for upstream and runner problems unrelated to the change. Frappe tests continue to run locally. Remove release.yml and .releaserc.json. tag-and-promote-from-pr-label.yml is now the only owner of tags, releases and promotion; running semantic-release alongside it made both tag the same version at different commits. (cherry picked from commit 8c6dd72)
Mechanical reformat produced by the standardised hooks. No behaviour change. (cherry picked from commit ac18190)
PyPDF2 3.0.1 carries PYSEC-2026-1835: a crafted PDF sends __parse_content_stream into an infinite loop, pinning a core, which text extraction can reach. PyPDF2 is no longer maintained and the fix ships in pypdf 3.9.0. The jubilee claim module already imports pypdf, so this makes the NHIF module match. PdfFileWriter is PdfWriter under the new name; nothing else changes. (cherry picked from commit 0315832)
admission.py called the translation function _ without importing it, so the missing admission number path raised NameError instead of the intended message. insurance.py evaluated a bare item_query statement that did nothing. nurse_record.py used _ as a throwaway loop variable, shadowing the translation function for the rest of that scope. (cherry picked from commit 7e69b6a)
Three names were defined twice in the same scope. Python already used the second definition in every case, so removing the dead first one changes no behaviour. - render_doc_as_html: the earlier definition lacked the use_setttings parameter and was never reachable. The whitelisted entry point is unaffected. - delete_medical_record: imported and then redefined locally; the import was dead. - validate_reqd_fields: the two copies were identical. (cherry picked from commit 174dc3a)
F401, F403 and F405 fire throughout the NHIF modules, which import helpers with star imports. av_tools and icd_tz already ignore these three, so this brings hms_tz onto the same policy rather than rewriting the imports. (cherry picked from commit 7076ef2)
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
2 participants
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.
Backport of #1503 to
version-15.Brings this app onto the shared toolchain so every in-house app enforces the
same rules.
Hooks now run at three stages
pre-commitcommit-msgpre-pushThe push gate runs
pre-commit run --all-files, so one push covering severalcommits is checked in full without slowing down each individual commit. It was
verified against a throwaway remote: a file committed with
--no-verifyiscaught and the push is rejected with nothing landing on the remote.
Run
bash scripts/setup-git-hooks.shonce per clone. It installs pre-commit ifit is missing and wires up all three hooks. Git cannot force hooks onto a fresh
clone, so the
Pre-commitworkflow runs--all-fileson every pull request asthe real gate.
Standardised
autopep8 and autoflake
environment and clones the rules relative to the repo
excluderegex. The oldnode_modules|.gitpattern also matched.github, so no workflow YAML was ever validated.Removed
ci.yml. Building a bench and migrating a throwaway site cost five to eightminutes per pull request and went red for upstream and runner problems
unrelated to the change. Frappe tests continue to run locally.
release.ymland.releaserc.json.tag-and-promote-from-pr-label.ymlis nowthe only owner of tags, releases and promotion. Running semantic-release
alongside it made both tag the same version at different commits, which fails
the promotion job.
Before merging
Check branch protection. If a required status check names
CI / Server TestsorCI / Install Smoke Check, that check no longer exists and will block every pullrequest until the rule is updated.
Pre-commit will be red. 51 pre-existing ruff findings remain, mostly
F405from star imports inhealthcare_service_request.py, plus 2F821undefined names and 9B006mutable argument defaults. Either fix them or widen the ignore list to match av_tools and icd_tz, in a separate pull request.