Skip to content

chore: adopt clang-format and pre-commit - #1128

Closed
prompterror wants to merge 1 commit into
boykopovar:mainfrom
prompterror:chore/pre-commit-tooling
Closed

prompterror wants to merge 1 commit into
boykopovar:mainfrom
prompterror:chore/pre-commit-tooling

Conversation

@prompterror

Copy link
Copy Markdown

What

Adopts a formatting/tooling baseline for the whole tree so style is enforced mechanically instead of by review:

  • .clang-format — LLVM base + deliberate deviations, all documented in the file: IndentWidth: 4 (the tree is 4-space indented), ColumnLimit: 120 (long shader-dump/profile lines are atomic; the tree already has 500-char lines), PointerAlignment: Left (matches the tree), SortIncludes: Never (include order is load-bearing here), FixNamespaceComments: false.
  • .editorconfig — LF, UTF-8, 4-space indent, final newline, no trailing whitespace; mirrors the hooks.
  • .pre-commit-config.yaml — pre-commit/pre-commit-hooks v6.0.0 (trailing-whitespace, end-of-file-fixer, check-yaml, check-merge-conflict, check-case-conflict, check-added-large-files, check-executables-have-shebangs, mixed-line-ending → LF) and pre-commit/mirrors-clang-format v21.1.8. Vendored code under 3rdparty/ is excluded from every hook.
  • .github/workflows/pre-commit.yml — CI job that runs pre-commit run --all-files on push to main and on pull requests, matching the existing Build workflow's checkout conventions.
  • CONTRIBUTING.md — documents pre-commit install and the format requirement.
  • Baseline reformat of core/ — clang-format -i (21.1.8, the pinned revision). Mechanical: line-wrapping at 120 columns, brace/indent normalization, single-statement blocks. No semantic changes (the working tree before this commit was already hook-clean, so CI --all-files passes).

The reformatting commit includes GuestMemory.cpp/ShaderMemory.cpp, which #1114 also touches — whichever merges second needs only a trivial rebase (git clang-format can't auto-merge formatting-only hunks, but the conflict is mechanical).

Testing

  • pre-commit run --all-files passes locally with the pinned revisions (provides the exact baseline CI will check).
  • Reformatting is whitespace/line-break only; verified there are no whitespace-insensitive content changes beyond the two minor fixes clang-format makes (if (x) return y; → two lines, one ( " typo in a profile string).

Note: CI runs on this fork PR are held by the first-time-contributor approval gate until a maintainer approves once (same as #1114).

Written with AI assistance; the config deviations and the baseline are reviewed per CONTRIBUTING.md.

Tooling baseline for the whole tree:

- .clang-format: LLVM base, 4-space indent, 120-column limit (long shader
  and profile lines stay atomic), pointer alignment left, include sorting
  off (include order is load-bearing here), namespace comments off.
- .editorconfig: lf, utf-8, 4-space indent, final newline, no trailing
  whitespace; mirrors the hooks.
- .pre-commit-config.yaml: pre-commit-hooks v6.0.0 (trailing-whitespace,
  end-of-file-fixer, check-yaml, check-merge-conflict, check-case-conflict,
  check-added-large-files, check-executables-have-shebangs, mixed-line-ending
  lf) and mirrors-clang-format v21.1.8. Vendored 3rdparty/ is excluded.
- .github/workflows/pre-commit.yml: CI job enforcing `pre-commit run
  --all-files` on push and pull_request.
- CONTRIBUTING.md: documents the hooks and the format requirement.
- Reformat core/ with clang-format 21.1.8. Mechanical, no semantic changes
  (e.g. git diff -w on the reformatting commit is smallest possible).
This was referenced Oct 7, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Thanks for the work here. This is a tree-wide policy change, so it needs a maintainer decision before a code review:

  • @boykopovar @oneandonlydean: do we want clang-format + a pre-commit CI job enforced on the whole tree? CONTRIBUTING doesn't have this rule today, and the baseline touches 1203 files (+82k/-50k).
  • A one-shot reformat conflicts with nearly every open PR (this branch already conflicts with main, and fix(agc): cache Linux /proc/self/maps regions for describePages #1114 hit the same on GuestMemory.cpp/ShaderMemory.cpp). If accepted, it would need a merge window where open PRs are rebased/merged first, or a git blame ignore-revs file plus formatting only changed lines (git clang-format) as the CI check.
  • Author: please rebase on main and keep the baseline reformat in a separate PR from the config/CI files, so the config can be discussed without a 80k-line diff.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

No from me: there's no formatter rule in CONTRIBUTING, and a whole-tree reformat would conflict with almost every open PR. I'd close this unless the owner wants one.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: a whole-tree reformat conflicts with nearly every open pull request, and we don't want a formatter rule for now. Thanks for the work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* ci .github docs libc libkernel needs rebase Conflicts with main, rebase needed prx relinker shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants