Repository navigation
style: adopt the app standards (managed config and formatting) - #7
Merged
Merged
Conversation
Add [tool.frappe-nix] (Avunu profile, Frappe 16, frappe-nix pinned to main until release-1 exists) and commit what `frappe-init --sync` renders: the flake and .envrc, the prek config, the ruff/ty/coverage tables, oxlint, oxfmt, stylelint, tsconfig, release-please, committed, the tools and workspace locks, and scripts/vite-register.mjs for the Vite font build. eslint.config.mjs, the eslint devDependencies and the remote ruff, prettier and eslint hooks are retired. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F
oxfmt re-wraps Jinja and microtemplate tags and splits translatable strings across lines, which changes their translation keys, so esign/**/*.html goes on the oxfmt ignore list through [tool.frappe-nix.js.oxfmt]. The managed zizmor hook reads .github/zizmor.yml, which frappe-nix's CI module does not render yet; this app-owned copy carries the profile's pinning and cooldown policy until it does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F
ssort, ruff safe fixes and ruff format (Python 3.14 target: unquoted annotations, unparenthesised except tuples), oxfmt at 110 columns with tabs, stylelint autofixes and test_utils copyright headers. Hand fixes for findings with no safe autofix, all behaviour-preserving: - ruff SIM103/SIM105/SIM108/SIM201/SIM210: bool() return, contextlib.suppress, a ternary, != and bool() in place of True if ... else False; F722 noqa on the generated DF.Select[""] line; - oxlint: if statements for expression-statement ternaries and &&, addEventListener for one-shot FileReader and Image handlers, two shadowed names renamed, and scoped disables for the side-effect imports and the self-rendering WebFormList; - test_utils static_analysis: a frappe-vulture:ignore Jinja comment on the four references in esign.html to frappe's own templates, which only resolve on a bench; - stylelint: overflow shorthands, kebab-case keyframe names (used only in this file) and the redundant -webkit-animation-name lines dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F
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.
Summary
Opts esign in to the app standards through the Avunu profile (
github:Avunu/frappe-standards-profile/main, Frappe 16) and commits the managed filesfrappe-init --syncrenders. This bootstraps the repo's firstflake.nix,.envrcandnix/uv.lock. The PR also retires eslint and prettier, applies the mechanical formatting pass, and makes small hand fixes for the stricter lint rules. None of it should change behaviour.What changed
Commit 1,
build: opt in … and sync managed files(sync output only):pyproject.toml:[tool.frappe-nix]withprofile,frappe-major = 16anddev-shell.frappe-nix-urlpinned to frappe-nix maine95f67funtilrelease-1exists;requires-python >=3.14(frappe v16 requires>=3.14,<3.15);[tool.bench.frappe-dependencies] frappe = ">=16.0.0,<17.0.0"; ty; coverage with thefail_under = 0seed; vulture and test_utils tables.package.json: the eslint devDependencies are removed (sync refuses eslint and prettier packages whilejsis on). The managed keys and scripts are added.buildis nowvite build && node scripts/vite-register.mjs.flake.nix,flake.lock,.envrc,nix/uv.lock;tools/pyproject.tomlandtools/uv.lock;.oxlintrc.json,.oxfmtrc.jsonc,.stylelintrc.json;tsconfig.jsonandtsconfig.desk.json;committed.toml,release-please-config.json,.release-please-manifest.json;.git-blame-ignore-revs(seed),marketplace/semgrep-baseline.json,scripts/vite-register.mjs..pre-commit-config.yaml. It now runs prek with pre-commit-hooks, ssort with the narrow controller exclude, local ruff viatools/uv.lock, oxfmt, oxlint, stylelint, zizmor, actionlint, shellcheck, compat, committed and test_utils..editorconfig;.gitignoremanaged block;esign/__init__.py.eslint.config.mjs, and the remote ruff, prettier and eslint hooks.Commit 2,
build: keep the HTML templates out of oxfmt and add the zizmor config:[tool.frappe-nix.js.oxfmt] ignore = ["esign/**/*.html"]. oxfmt was re-wrapping the Jinja pageesign/templates/esign.htmland the timeline microtemplate. It split_("Sign & Submit", context="Button in web form")and_("{0} if you are not redirected …")across lines, which changes their translation keys. The old prettier hook never touched HTML either..github/zizmor.yml. The managed zizmor hook reads this file, but frappe-nix's CI module doesn't render it yet. This app-owned copy carries the profile's policy (hash-pin third-party actions, 3-day cooldown) until it does.Commit 3,
style: apply the managed formatting and lint pass:config/__init__.pyandcustom/web_form.py; definitions now come before their first use;except A, B:(PEP 758) inweb_form.py'saccept();vite.config.mjsand the Markdown;return bool(esign_web_form);contextlib.suppress;!=;bool(...);# noqa: F722on the hand-writtenDF.Select[""]line.&&becomeifstatements;FileReader/Imageonload/onerrorhandlers becomeaddEventListener;e→eventinupload.js, and thesetup_fieldsparameter →form_doc;fonts.jsandesign.web.bundle.jsand the self-renderingnew WebFormList(...).-webkit-animation-nameduplicates are dropped, since it is an alias ofanimation-name.static_analysis: a{# frappe-vulture:ignore … #}Jinja comment on the fouresign.htmllines that extend or include frappe's own templates. These only resolve on a bench; see follow-ups.Verification
Run at head
db24b9c.Inside the dev shell (
nix develop --no-pure-eval), against the committed lock (profile at9776af8, after #5), with no--profile-path:frappe-init --syncran clean, includingnix run .#relock.frappe-nix sync --check→frappe-nix: clean, exit 0.frappe-nix compat→ exit 0.SKIP=static_analysis prek run --all-files→ every hook passes (the dev shell does not putprekon PATH, so it came from outside:nix build nixpkgs#prek, 0.5.5), frappe-nix-compat included. static_analysis is skipped because in the dev shell it walks.frappe-nix/and.devenv/and reports 371 errors, all in frappe's own files. fix(standards): keep the managed hooks out of the dev bench and venv frappe-nix#104 fixes that.committed origin/develop..HEAD→ exit 0.frappe-test --recreate-site:fail_underfrom the seed 0 to 27. That is PR B work.yarn build(Vite fonts) buildsesign-fonts.cssand the font files as before.node --checkpasses on every JS file, andpy_compilepasses on every Python file.bench build --app esignfails here onesign.web.bundle.js's../../../../../frappe/...relative imports, because in the dev shellapps/esignis a link to this checkout. Those import lines are unchanged by this PR, so the bundles were not compiled locally. See follow-ups.Outside the dev shell, on a fresh
git cloneof the branch (no.frappe-nix/or.devenv/, withyarnfrom nixpkgs):prek run --all-files→ every hook passes, static_analysis included. frappe-nix-compat skips itself there by design.Other checks:
git diff -w --stat origin/develop..HEAD, excluding locks: 44 files, +1539/−1138. Nearly all of it is managed configs, the ssort moves (a sorted-line comparison shows the Python changes are pure reorders, plus headers and the 3.14 syntax) and oxfmt reflow.git diff -won the style commit: 22 files.Review guide
pyproject.toml: the[tool.frappe-nix]table, the oxfmt ignore and the stricter ruff config, which drove the hand fixes.esign/esign/custom/web_form.pyis mostly an ssort move. Usegit diff -w --color-moved. The only edited lines are the SIM201/SIM108/SIM210 rewrites inaccept()/get_esign_link, and oneexceptline in 3.14 syntax inaccept().esign/esign/__init__.py: two hand-edited spots, the SIM105contextlib.suppress(json.JSONDecodeError, TypeError)(wastry/except (…): pass) and the SIM103return bool(esign_web_form).esign/public/js/controls/signature.jsandupload.js(theaddEventListenerswaps), andesign/public/js/web/esign_webform_script.js(theifrewrites, theform_docrename and theno-newdisable).esign/public/css/esign.web.bundle.scss: the toast keyframes were renamed. Check that the bottom-centre toast animation still plays on the signing page.requires-python >=3.14. Theexcept A, B:syntax only parses on 3.14+. frappe v16 already requires 3.14, but this ends Python-level support for v15 benches.hooks.pystill has its v15 branch, which PR B removes (movingoverride_doctype_classtoextend_doctype_class).package.jsonnow has"type": "module".vite.config.mjswas already ESM, and the font build is verified.direnv allow(ornix develop --no-pure-eval), thenfrappe-init --checkandfrappe-test --recreate-site. The dev shell does not provideprek, so bring it from outside:nix run nixpkgs#prek -- run --all-fileswithSKIP=static_analysisinside the shell, or the full run on a clean checkout outside it. Then sign a document through an eSign web form on a bench wherebench buildworks.Not done / follow-ups
.git-blame-ignore-revsin PR B (frappe-nix compat --add-blame-ignore <sha>).args: ["."]and no--dependency-paths, so{% extends %}/{% include %}of frappe's own templates are "not found" on a clean checkout. With--dependency-paths <frappe>it passes unchanged. The four markers here can go once the managed config passes frappe (and the siblings) as dependency paths.prek. The managed config is a prek config, but the frappe-nix dev shell doesn't putprekon PATH; it should..github/zizmor.ymlis not rendered. Once the CI module renders it, sync should take over the app-owned copy..oxfmtrc.jsonccould ignore<app>/**/*.html(Jinja and microtemplates) by default.esign.web.bundle.jsimports frappe sources by relative path (../../../../../frappe/...). That resolves only when the app physically sits inapps/, sobench buildfails in the frappe-nix dev shell. Switch the imports to a bench-independent form in PR B.docs.ymlstays as is (the profile hasdocs-siteoff, and the docs are built by Avunu/site-docs).fail_under0 → 27 (the ratchet's figure), tests for the 15 testmap gaps (the signature flow and the request/session paths first), ty clean, removing the v15override_doctype_classbranch, desk JS under strict checkJs (tsconfig.desk.jsoncoversesign/**/*.js;yarn run typecheckis expected to report errors until then), the four remainingno-consolewarnings inesign_webform_script.js, and building the font CSS as a named bundle registered through vite-register instead of the fixed/assets/esign/dist/esign-fonts.csspath.esign_webforms(the Python package) is a separate, later PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F