Conversation
Resolves modular#6885 Addresses modular#1249 Signed-off-by: Kavindu Sachinthe <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Mojo’s public-facing tooling configuration and release notes to expose non-mutating formatting checks via mojo format --check (and optional --diff) for CI/pre-commit workflows.
Changes:
- Add a
format-checkPixi task that runsmojo format --checkover./stdlib. - Document
--check/--diffbehavior and exit-code semantics in the nightly changelog. - Add a new pre-push pre-commit hook intended to run non-mutating checks before pushing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| mojo/pixi.toml | Adds a format-check task invoking mojo format --check for CI-friendly formatting validation. |
| mojo/docs/nightly-changelog.md | Adds release-note documentation for mojo format --check/--diff and exit codes. |
| .pre-commit-config.yaml | Adds a pre-push hook to run Bazel-based checks prior to pushing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+25
to
+27
| - id: format-check | ||
| name: format-check | ||
| entry: ./bazelw run lint |
Comment on lines
+110
to
+113
| - `mojo format` now supports a `--check` mode that reports which files | ||
| would be reformatted and returns a non-zero exit code without modifying | ||
| any source files. Use `--diff` alongside `--check` to display a unified | ||
| diff. This enables race-free CI formatting checks: |
Implements the feature requested in modular#6885 by wrapping mblack (the underlying formatter used by mojo format) directly in check mode. Since mojo format is a pre-compiled closed-source binary that does not yet expose --check, this script provides the feature today using the same mblack package that the binary itself uses internally. Usage: python3 utils/mojo-format-check.py src/ tests/ Exit codes match the mblack/black spec: 0 all files already formatted 1 one or more files would change (no files modified) 123 internal formatter error Also updates pixi.toml and .pre-commit-config.yaml to use this script. Signed-off-by: Kavindu Sachinthe <[email protected]>
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.
What this PR does
This PR delivers the
--checkmode feature in two layers:Layer 1 — Working TODAY:
utils/mojo-format-check.pyA new script that implements the full requested behavior right now, without
requiring any changes to the closed-source
mojobinary.It works by calling
mblackdirectly — the same underlying formatter thatmojo formatuses internally — but in check mode:This is already wired up to:
pixi run format-check(viamojo/pixi.toml)pre-pushpre-commit hook (via.pre-commit-config.yaml)Layer 2 — Needs Modular internal work:
mojo format --checkThe real fix the issue requests —
mojo format --checkworking as a nativeCLI flag — requires modifying the
mojodriver binary (C++ source in theprivate KGEN repo). This PR cannot do that, but the implementation is
straightforward:
C++ driver changes needed (for Modular engineers)
Step A — Register the CLI options:
Step B — Forward to mblack subprocess:
Step C — Do not remap the exit code. The current driver may eat nonzero
exit codes from mblack. The
--checkcontract depends on these beingpropagated faithfully:
0→ already formatted1→ would reformat123→ internal error (inherited from black's spec)Once the binary supports
--check, thepixi.tomltask and pre-commit hookcan be updated to use
mojo format --checkinstead of the wrapper script.Files changed
utils/mojo-format-check.pymojo/pixi.tomlformat-checktask.pre-commit-config.yamlformat-checkpre-push hookmojo/docs/nightly-changelog.md