Skip to content

Conversation

@namurphy
Copy link
Contributor

@namurphy namurphy commented Nov 20, 2025

This PR tests out adding mdformat as a pre-commit hook to apply a consistent formatting to Markdown files. It includes a few extensions to mdformat to account for GitHub flavored Markdown, etc.

I've tentatively excluded PHEP 1, PHEP 2, and standards.md. If we want to apply these changes to existing PHEPs, we probably want to do it in a separate PR or when we make revisions. Of these, it seems that PHEP 2 would be the one most worth updating since it is the PHEP template.

If this PR is accepted, then PHEPs in progress could apply changes by commenting pre-commit.ci autofix in the conversation tab of their pull request or by running pre-commit run --all-files locally, or uvx pre-commit run --all-files if uv is installed.

By default, mdformat will change numbered lists to all start with 1., so as to minimize diffs. I set the configuration to make these lists numbered instead in case we make the Markdown file the version of record.

@namurphy
Copy link
Contributor Author

pre-commit.ci autofix

@namurphy namurphy force-pushed the pre-commit-mdformat branch from 72e7e40 to 2146ca0 Compare November 20, 2025 18:50
Comment on lines +54 to +55
- mdformat-gfm
- mdformat-gfm-alerts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gfm refers to GitHub flavored Markdown.

@namurphy namurphy marked this pull request as ready for review November 20, 2025 18:53
@namurphy
Copy link
Contributor Author

namurphy commented Nov 20, 2025

@sapols — does this seem reasonable to you? If so, we'd want to apply mdformat to the PHEP PRs that are about to be merged beforehand. Thanks!

- mdformat-gfm
- mdformat-gfm-alerts
- mdformat-ruff
language_version: python3.14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little wary of this; I was unable to run this pre-commit stuff locally because my machine doesn't have 3.14 yet. It seems like the exclude feature only works on Python 3.13+ though so I see why you added it.

@sapols
Copy link
Contributor

sapols commented Jan 8, 2026

Thanks @namurphy. Sorry for the super slow response, crazy holiday season.

I'm not sure about this one... I personally don't see much benefit. Feels to me like it's solving a problem that didn't exist (I never considered our markdown formatting problematic). In fact, I could see this causing problems where people don't know about these opinionated markdown rules, causing their PRs to have failing checks they might not understand. Sure the checks could be fixed easily enough by commenting pre-commit.ci autofix or running pre-commit run --all-files locally (if you have Python 3.14), but I'm just not sure it's worth that headache.

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.

2 participants