docs: require formatting for every contribution#527
Conversation
Make `npm run format:write` a hard pre-commit/PR gate rather than advisory guidance, and clarify it applies to every file in the repo (not just docs content) so generated scaffolding under plans/, scripts/, etc. is not missed. Add a dedicated Formatting section to CONTRIBUTING.md noting CI enforces format:check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates AGENTS.md and CONTRIBUTING.md to clarify that code formatting is a strict requirement before submitting a pull request, detailing the commands to format and check files, and explaining that CI will fail on unformatted code. The review comments suggest improving the documentation by explicitly listing .ts and .json file extensions in CONTRIBUTING.md to align with AGENTS.md, and clarifying the workflow steps (including pushing changes) when a CI formatting check fails.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| ## Formatting | ||
|
|
||
| **Every contribution must be formatted before it is committed.** This applies to _all_ files in the repository — content (`.md`/`.mdx`), TypeScript, config, and anything else under `plans/`, `scripts/`, etc. — not just docs content. |
There was a problem hiding this comment.
To ensure consistency with the file types listed in AGENTS.md (which explicitly mentions .ts and .json), we can explicitly list these extensions here as well. This makes it clearer to contributors which file formats are subject to formatting checks.
For example:
**Every contribution must be formatted before it is committed.** This applies to _all_ files in the repository — content (`.md`/`.mdx`), TypeScript (`.ts`), JSON (`.json`), config, and anything else under `plans/`, `scripts/`, etc. — not just docs content.| npm run format:check | ||
| ``` | ||
|
|
||
| CI runs `npm run format:check` on every pull request and will fail if anything is unformatted. If CI fails on formatting, run `npm run format:write` locally and commit the result. Prettier is configured via `@harperdb/code-guidelines/prettier` (see `package.json`). |
There was a problem hiding this comment.
When CI fails on formatting, the contributor needs to run the format command, commit, and then push the changes back to the branch. Adding "and push" to the instructions makes the workflow complete and clear for contributors.
For example:
If CI fails on formatting, run `npm run format:write` locally, commit, and push the changes.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-527 This preview will update automatically when you push new commits. |
dawsontoth
left a comment
dawsontoth
left a comment
There was a problem hiding this comment.
I'm guessing this is to help the agents get it right before CI fails, and https://github.com/HarperFast/documentation/blob/main/.github/workflows/validate.yaml is working well too? As a harder gate to actually formatting.
|
Yes! I also opened #528 if we want to consider automating this as a commit hook. |
🧹 Preview CleanupThe preview deployment for this PR has been removed. |
2 participants
Summary
Formatting was already documented, but as advisory guidance rather than a hard gate — which is how unformatted files (the
plans/scaffolding in #520) slipped through. This tightens the contribution docs so future contributions, including agentic ones, treat formatting as a non-negotiable pre-PR step.npm run format:write/format:checka hard pre-commit/PR gate (not advisory), and clarify it applies to every file in the repo, not just docs content (so generated files underplans/,scripts/, etc. aren't missed).format:check.🤖 Generated with Claude Code