Skip to content

style: adopt the app standards (managed config and formatting) - #7

Merged
batonac merged 3 commits into
developfrom
style/adopt-app-standards
Oct 17, 2026
Merged

batonac merged 3 commits into
developfrom
style/adopt-app-standards

Conversation

@batonac

@batonac batonac commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

Summary

Opts esign in to the app standards through the Avunu profile (github:Avunu/frappe-standards-profile/main, Frappe 16) and commits the managed files frappe-init --sync renders. This bootstraps the repo's first flake.nix, .envrc and nix/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:
    • new [tool.frappe-nix] with profile, frappe-major = 16 and dev-shell.frappe-nix-url pinned to frappe-nix main e95f67f until release-1 exists;
    • sync's managed key groups: the profile's ruff config replaces the loose one and drops the blanket B017/B018/B023/B904/E402/E741/F403/F405/F722 ignores; 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 the fail_under = 0 seed; vulture and test_utils tables.
  • package.json: the eslint devDependencies are removed (sync refuses eslint and prettier packages while js is on). The managed keys and scripts are added. build is now vite build && node scripts/vite-register.mjs.
  • New files:
    • flake.nix, flake.lock, .envrc, nix/uv.lock;
    • tools/pyproject.toml and tools/uv.lock;
    • .oxlintrc.json, .oxfmtrc.jsonc, .stylelintrc.json;
    • tsconfig.json and tsconfig.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.
  • Rewritten:
    • .pre-commit-config.yaml. It now runs prek with pre-commit-hooks, ssort with the narrow controller exclude, local ruff via tools/uv.lock, oxfmt, oxlint, stylelint, zizmor, actionlint, shellcheck, compat, committed and test_utils.
    • .editorconfig;
    • the .gitignore managed block;
    • the release-please markers in esign/__init__.py.
  • Retired: 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 page esign/templates/esign.html and 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:

  • Automatic changes:
    • ssort reorders config/__init__.py and custom/web_form.py; definitions now come before their first use;
    • ruff safe fixes for the 3.14 target: unquoted annotations, and one unparenthesised except A, B: (PEP 758) in web_form.py's accept();
    • ruff format;
    • oxfmt at 110 columns with tabs, which reflows JS, CSS, vite.config.mjs and the Markdown;
    • stylelint autofixes;
    • test_utils copyright headers.
  • Hand fixes for findings with no safe autofix. All of them preserve behaviour:
    • ruff:
      • SIM103 → return bool(esign_web_form);
      • SIM105 → contextlib.suppress;
      • SIM108 → a ternary;
      • SIM201 → !=;
      • SIM210 → bool(...);
      • # noqa: F722 on the hand-written DF.Select[""] line.
    • oxlint:
      • expression-statement ternaries and && become if statements;
      • one-shot FileReader/Image onload/onerror handlers become addEventListener;
      • two shadowed names are renamed: e → event in upload.js, and the setup_fields parameter → form_doc;
      • scoped disables, each with a reason, cover the side-effect imports in fonts.js and esign.web.bundle.js and the self-rendering new WebFormList(...).
    • stylelint:
      • overflow shorthands with the same x/y values;
      • kebab-case keyframe names, which are referenced only inside that SCSS file;
      • the -webkit-animation-name duplicates are dropped, since it is an alias of animation-name.
    • test_utils static_analysis: a {# frappe-vulture:ignore … #} Jinja comment on the four esign.html lines 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 at 9776af8, after #5), with no --profile-path:

  • frappe-init --sync ran clean, including nix 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 put prek on 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:
    • the app installs cleanly on a fresh v16 site;
    • composition passes ("2 controller(s) compose in both orders"), so the reordered Web Form and Email Template extensions load in both install orders;
    • the app has no tests yet. Coverage from the install hooks is 28.7%, and the testmap lists 15 untested whitelisted methods and hooks. Its exit status is 2, because the ratchet asks to raise fail_under from the seed 0 to 27. That is PR B work.
  • yarn build (Vite fonts) builds esign-fonts.css and the font files as before.
  • node --check passes on every JS file, and py_compile passes on every Python file.
  • bench build --app esign fails here on esign.web.bundle.js's ../../../../../frappe/... relative imports, because in the dev shell apps/esign is 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 clone of the branch (no .frappe-nix/ or .devenv/, with yarn from 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 -w on the style commit: 22 files.

Review guide

  1. Start with pyproject.toml: the [tool.frappe-nix] table, the oxfmt ignore and the stricter ruff config, which drove the hand fixes.
  2. esign/esign/custom/web_form.py is mostly an ssort move. Use git diff -w --color-moved. The only edited lines are the SIM201/SIM108/SIM210 rewrites in accept()/get_esign_link, and one except line in 3.14 syntax in accept().
  3. esign/esign/__init__.py: two hand-edited spots, the SIM105 contextlib.suppress(json.JSONDecodeError, TypeError) (was try/except (…): pass) and the SIM103 return bool(esign_web_form).
  4. JS: esign/public/js/controls/signature.js and upload.js (the addEventListener swaps), and esign/public/js/web/esign_webform_script.js (the if rewrites, the form_doc rename and the no-new disable).
  5. 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.
  6. Risks:
    • requires-python >=3.14. The except A, B: syntax only parses on 3.14+. frappe v16 already requires 3.14, but this ends Python-level support for v15 benches. hooks.py still has its v15 branch, which PR B removes (moving override_doctype_class to extend_doctype_class).
    • package.json now has "type": "module". vite.config.mjs was already ESM, and the font build is verified.
  7. To test: direnv allow (or nix develop --no-pure-eval), then frappe-init --check and frappe-test --recreate-site. The dev shell does not provide prek, so bring it from outside: nix run nixpkgs#prek -- run --all-files with SKIP=static_analysis inside the shell, or the full run on a clean checkout outside it. Then sign a document through an eSign web form on a bench where bench build works.

Not done / follow-ups

  • Blame ignore: please add this PR's squash SHA to .git-blame-ignore-revs in PR B (frappe-nix compat --add-blame-ignore <sha>).
  • frappe-nix gaps found here:
    • static_analysis has no dependency paths. The managed hook runs with 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.
    • The dev shell has no prek. The managed config is a prek config, but the frappe-nix dev shell doesn't put prek on PATH; it should.
    • .github/zizmor.yml is not rendered. Once the CI module renders it, sync should take over the app-owned copy.
    • oxfmt formats Jinja templates. The managed .oxfmtrc.jsonc could ignore <app>/**/*.html (Jinja and microtemplates) by default.
  • Web bundle build in the dev shell: esign.web.bundle.js imports frappe sources by relative path (../../../../../frappe/...). That resolves only when the app physically sits in apps/, so bench build fails in the frappe-nix dev shell. Switch the imports to a bench-independent form in PR B.
  • CI: the CI caller workflows are not rendered yet, because the reusable workflows haven't landed. docs.yml stays as is (the profile has docs-site off, and the docs are built by Avunu/site-docs).
  • PR B items: fail_under 0 → 27 (the ratchet's figure), tests for the 15 testmap gaps (the signature flow and the request/session paths first), ty clean, removing the v15 override_doctype_class branch, desk JS under strict checkJs (tsconfig.desk.json covers esign/**/*.js; yarn run typecheck is expected to report errors until then), the four remaining no-console warnings in esign_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.css path.
  • Rename to esign_webforms (the Python package) is a separate, later PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F

batonac and others added 3 commits October 16, 2026 22:08
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
@batonac
batonac merged commit 0f2bacb into develop Oct 17, 2026
2 checks passed
@batonac
batonac deleted the style/adopt-app-standards branch October 17, 2026 03:33
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.

1 participant