ci: standardise pre-commit, lint and release tooling - #132
Merged
av-dev2 merged 3 commits intoAug 26, 2026
Merged
Conversation
Brings version-16-hotfix onto the same toolchain as version-15-hotfix. Adds 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. Adds the pre-commit and semantic-commits workflows, commitlint, and scripts/setup-git-hooks.sh. Anchors the exclude regex so .github/ is no longer skipped by an unanchored .git pattern. Removes ci.yml, release.yml and .releaserc.json. tag-and-promote-from-pr-label.yml is now the only owner of tags, releases and promotion.
stock_entry was assigned on self inside on_submit. Frappe has already written the row by then, so the assignment was discarded and the link field stayed empty. Now written with db_set. orderby passed Order.asc positionally, so pypika read it as a second ORDER BY column instead of a direction, in five queries. get_value on a Single doctype is not type safe; switched to get_single_value. send_email was compared against True. Frappe returns checkbox fields as the integer 1, so this is now a plain truthiness test. A bare except around json.loads now catches ValueError.
Mechanical ruff reformat, plus ruff autofixes for printf style formatting, unused variables and unnecessary allocations. Five semgrep findings waived, each naming the rule and the reason in the code: frappe-ssti (hardcoded template path, static template), frappe-codeinjection-eval (Script Report python needs real imports; editing a Report is restricted to System Manager) and frappe-breaks-multitenancy on three query builder tables in one report.
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.
The automatic backport of #130 to
version-16-hotfixcould not cherry-pick,because that pull request contains a repository-wide reformat. This branch
applies the same standard to
version-16-hotfixdirectly instead.Hooks now run at three stages
pre-commitcommit-msgpre-pushRun
bash scripts/setup-git-hooks.shonce per clone. Git cannot force hooksonto a fresh clone, so the
Pre-commitworkflow running--all-fileson everypull request is the real gate.
Removed
ci.yml,release.ymland.releaserc.json.tag-and-promote-from-pr-label.ymlis now the only owner of tags, releases andpromotion.
Defects fixed along the way
stock_entrywas assigned onselfinsideon_submit, after Frappe hadwritten the row, so the link field stayed empty. Now uses
db_set..orderby(sd.salary_component, Order.asc)passed the direction positionally,so pypika read it as a second ORDER BY column. Five queries.
get_valueon a Single doctype is not type safe; nowget_single_value.send_email == Truecompared a Frappe checkbox againstTrue. Frappe returnsthe integer
1, so this is now a truthiness test.exceptaroundjson.loadsnow catchesValueError.Semgrep waivers
Five findings reviewed and waived, each naming the rule and the reason in the
code:
frappe-ssti(hardcoded template path, static template),frappe-codeinjection-eval(Script Report python needs real imports, sosafe_execis not usable; editing a Report is restricted to System Manager) andfrappe-breaks-multitenancyon three query builder tables in one report.The
frappe-codeinjection-evalwaiver documents the risk rather than removingit. Revisit if Report write access is ever widened beyond System Manager.
pre-commit run --all-filespasses on this branch.