ci: adopt the standard pre-commit, ruff, and semgrep toolchain - #4
Merged
Conversation
Install hooks at three stages. pre-commit checks staged files only so commits stay fast, commit-msg runs commitlint against the conventional commit rules, and pre-push re-enters pre-commit over the whole repository so a push cannot carry unchecked commits. Anchor the exclude regex. The old "node_modules|.git" pattern was unanchored, so ".git" also matched ".github" and no workflow YAML was ever validated, and it hid COMMIT_EDITMSG from the commit-msg stage. Add the frappe-semgrep-rules hook as language: python with semgrep in additional_dependencies, so it installs its own semgrep and clones the rules into the repo on first run instead of depending on a path that only exists on one machine. Ignore the clone target. Add no-commit-to-branch for main, master, production, and the version branches. Working branches such as version-15-hotfix stay open. Own the ruff config in this app. line-length moves from 120 to 110, target stays py310, and the missing [tool.ruff.format] section is added with double quotes and tab indentation. E101 and W191 join the ignore list because W is selected and the app indents with tabs. An explicit [tool.ruff.lint] section stops ruff resolving upward to a config outside the app. Add the dev extra and scripts/setup-git-hooks.sh so a fresh clone can install the hooks in one command.
Remove ci.yml and ci-tests.yml. Both build a full bench on the runner (bench init, get-app, new-site, install-app, migrate) with MariaDB and two Redis services. Frappe tests run locally on the bench terminal, not in GitHub Actions, so these only added five to eight minutes per pull request and went red for upstream breakage unrelated to the diff. Remove release.yml and .releaserc.json. Tagging, releasing, and promotion belong to tag-and-promote-from-pr-label.yml. Running semantic-release alongside it tags a separate release commit, after which tag-and-promote finds the tag on a different commit and fails. Add pre-commit.yml so pre-commit runs over all files on every pull request, and semantic-commits.yml so commitlint re-checks the whole PR commit range. Both gates are server side and hold for contributors who never installed the local hooks. Rescope linter.yml to a full semgrep scan plus pip-audit. The blocking step now uses r/python.lang.security at ERROR severity, with a non-blocking WARNING pass for information. The duplicate pre-commit step is gone, since pre-commit.yml covers it over all files.
Use frappe.db.get_single_value for the four reads of Property Management Settings in issue_hook. That doctype is a single, and frappe.db.get_value is not type safe against a single. The rest of the file already reads singles this way. Initialise invoice_item in leaseInvoiceAutoCreate. The flush branch reads the previous iteration's row, which the row_num guard makes safe at runtime, but the name had no binding at the top of the loop and ruff reported it undefined. Drop three dead locals: foreign_currency in both rent invoice reports, which nothing reads after the branch assigns it, and name_in_json in create_property_setter. Replace explicit positional format indices and rename an unused loop variable.
Apply ruff at line-length 110 with tab indentation and double quotes across the app, together with the safe lint autofixes and the whitespace and end-of-file hooks. No behaviour changes.
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
1 participant
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.
No description provided.