Skip to content

ci: standardise pre-commit, lint and release tooling - #132

Merged
av-dev2 merged 3 commits into
Aakvatech-Limited:version-16-hotfixfrom
av-dev2:chore/ci-standardisation-version-16-hotfix
Aug 26, 2026
Merged

ci: standardise pre-commit, lint and release tooling#132
av-dev2 merged 3 commits into
Aakvatech-Limited:version-16-hotfixfrom
av-dev2:chore/ci-standardisation-version-16-hotfix

Conversation

@av-dev2

@av-dev2 av-dev2 commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

The automatic backport of #130 to version-16-hotfix could not cherry-pick,
because that pull request contains a repository-wide reformat. This branch
applies the same standard to version-16-hotfix directly instead.

Hooks now run at three stages

Stage Scope Behaviour
pre-commit staged files fast, autofixes, blocks the commit
commit-msg the message conventional prefix, blocks the commit
pre-push whole repository blocks the push on any failure

Run bash scripts/setup-git-hooks.sh once per clone. Git cannot force hooks
onto a fresh clone, so the Pre-commit workflow running --all-files on every
pull request is the real gate.

Removed

ci.yml, release.yml and .releaserc.json.
tag-and-promote-from-pr-label.yml is now the only owner of tags, releases and
promotion.

Defects fixed along the way

  • stock_entry was assigned on self inside on_submit, after Frappe had
    written 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_value on a Single doctype is not type safe; now get_single_value.
  • send_email == True compared a Frappe checkbox against True. Frappe returns
    the integer 1, so this is now a truthiness test.
  • A bare except around json.loads now catches ValueError.

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, so
safe_exec is not usable; editing a Report is restricted to System Manager) and
frappe-breaks-multitenancy on three query builder tables in one report.

The frappe-codeinjection-eval waiver documents the risk rather than removing
it. Revisit if Report write access is ever widened beyond System Manager.

pre-commit run --all-files passes on this branch.

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.
@av-dev2
av-dev2 merged commit e50b6fa into Aakvatech-Limited:version-16-hotfix Aug 26, 2026
4 checks passed
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