skrub-data
META - Discussing formatting changes for the repository
There are a few open PRs that are adding formatting rules to the pre-commit checks used when contributing to the repository.
In general, the maintainers' intention is to keep the barrier to contributing low, which means keeping the formatting checks on the simpler side. However, some checks should be performed no matter what to improve code quality and avoid some issues.
In this issue I am tracking formatting PRs. Some may have been closed as the suggestions to the formatting rules were not accepted (typically because they were too restrictive), but they may be reconsidered at a later point.
Discussions on formatting
Merged PRs
- #1712
- #1711
- #1707
- #1697
- #1694
Closed PRs
- #1706
- #1752
Ongoing PRs
- #1709
- #1700
- #1703
- #1696
- #1699
- #1753
- #1751
- #1705
- #1704
FWIW, I will be adding some of these rules to a local linting preset that I will be using for my code. There is no plan to add all of them to the required pre-formatting checks, however.
Note that pre-commit will format and apply a large part of the Ruff rules automatically, without requiring any action from contributors.
Note that pre-commit will format and apply a large part of the Ruff rules automatically, without requiring any action from contributors.
I was discussing yesterday with a maintainer from a major project, and he rightfully pointed out that pre-commit are a major security risk, in terms of code injection, dependency chain attacks, and audit. Security issues are real and important in today's world.
Is pre-commit more a security risk than GitHub actions, AI agents, and GitHub itself? It's a genuine question, I'm not sure how pre-commit is worse than GitHub, and couldn't find literature on the subject (but didn't spend time searching thoroughly). With that said, I'm not a huge fan of pre-commit either, one reason is that it fails from time to time on older (but still maintained) Linux distributions.
In any case, pre-commit predates the recent changes.