Skip to content

Fix ci precommit md060#40

Merged
fredclausen merged 2 commits into
mainfrom
fix-ci-precommit-md060
Jul 10, 2026
Merged

Fix ci precommit md060#40
fredclausen merged 2 commits into
mainfrom
fix-ci-precommit-md060

Conversation

@fredclausen

@fredclausen fredclausen commented Jul 10, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes

    • Improved Markdown checks to support hand-edited tables without unnecessary formatting warnings.
    • Prevented automatic formatting from rewriting Markdown and MDX tables.
  • Chores

    • Improved pre-commit checks on macOS by avoiding a platform-specific test that could cause validation failures.
    • Maintained Markdown style enforcement through dedicated linting rules.

nixpkgs' pre-commit derivation runs its own upstream pytest suite in
checkPhase, and test_output_isatty forks a pty-backed subprocess that
is racy under the sandboxed, multi-threaded macOS builders (see the
'use of fork() may lead to deadlocks' DeprecationWarning in the test
output). It fails intermittently on macos-latest CI runners even
though the built pre-commit binary works fine, and is not caused by
any change in this repo: the same nixpkgs-4.5.1 derivation succeeded
in other jobs of the same CI run.

Override pkgs.pre-commit on darwin to add that one test to
disabledTests (matching the pattern nixpkgs itself already uses for
other known-broken darwin tests), and pass the override to
git-hooks.lib.run via its package option. Linux is left untouched.
Safe to drop once upstream disables or fixes the test in
pkgs/by-name/pr/pre-commit/package.nix.
MD060 (table-column-style) has no config value that means 'ignore
table width' -- even the default 'any' style still requires every
table to consistently match one of three fixed padding conventions
(aligned/compact/tight) and reports violations against whichever is
the closest match. Disable it outright so hand-edited/ragged table
widths never fail the build.

Disabling MD060 alone isn't enough: prettier defaults to write=true
here and always re-pads GFM tables to its own 'aligned' style, with
no config knob to leave tables alone (a long-standing open upstream
feature request). In a CI-only setup with no local pre-commit hook
installed, that rewrite is an invisible diff contributors can only
find by digging through the CI build log, so it looks like an opaque
failure for something markdownlint no longer cares about. Exclude
markdown from the prettier hook entirely; markdownlint is check-only
(no --fix) and its default ruleset already flags the same style
concerns (emphasis style, bullet markers, blank lines, ...) as a
named, actionable rule failure instead of a silent rewrite.
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 60968055-3eb6-4086-81a0-aa8b498ee75c

📥 Commits

Reviewing files that changed from the base of the PR and between edc9ac4 and bc53889.

📒 Files selected for processing (2)
  • checks/base.nix
  • flake.nix

📝 Walkthrough

Walkthrough

The changes adjust Markdown formatting hooks and customize the pre-commit package used by Darwin checks.

Changes

Pre-commit adjustments

Layer / File(s) Summary
Markdown hook configuration
checks/base.nix
Markdownlint disables MD060, and Prettier excludes Markdown and MDX files.
Darwin pre-commit package
flake.nix
Darwin overrides pkgs.pre-commit to disable test_output_isatty, and lib.mkCheck passes the customized package to the hook runner.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adjusting CI pre-commit handling for markdownlint MD060.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-ci-precommit-md060

Comment @coderabbitai help to get the list of available commands.

@fredclausen fredclausen enabled auto-merge July 10, 2026 18:22
@fredclausen fredclausen merged commit a9541fa into main Jul 10, 2026
15 checks passed
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