[Numpy-discussion] Automatic formatters for only changed lines
Hey all, Matti pointed out to me that discussion of an autoformatter has come up before, with the decision that an entire codebase rewrite would be undesirable (ref: https://mail.python.org/pipermail/scipy-dev/2020-January/023927.html). As a result, tools/linter.py was added to only validate the changed lines. There is better support for autoformatters that only work on changed lines now, and I'd like to propose adding one. I already have a pre-commit demo implemented here, with an example run: https://github.com/numpy/numpy/pull/21449. It applies Black to changed python file lines plus flake8 to validate them (changed lines only), and clang-format with the pep7 config to any changed c/c++ lines. Pre-commit has the easy interface to run on staged files (default) all files, specific files, or files changed between --from-ref and --to-ref. I don't necessarily know that this should be enforced on CI off the bat (though in the future it could be), but at least having the tool available and easily usable would help to incrementally improve codebase style, and remove the need for formatting by hand. Any thoughts? - Trevor ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
[Numpy-discussion] Re: Automatic formatters for only changed lines
On Thu, May 5, 2022, at 12:00, Trevor Gross wrote: > I don't necessarily know that this should be enforced on CI off the bat > (though in the future it could be), but at least having the tool available > and easily usable would help to incrementally improve codebase style, and > remove the need for formatting by hand. On all projects we've set up pre-commit with style formatting, it has significantly reduced noise and nitpicks in PR discussions. It makes for a better contributor experience, thus I am in favor. Black isn't perfect by any means, but never having to talk about formatting again makes it worth it! Stéfan ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
[Numpy-discussion] Re: Automatic formatters for only changed lines
On 6/5/22 01:32, Stefan van der Walt wrote: On Thu, May 5, 2022, at 12:00, Trevor Gross wrote: I don't necessarily know that this should be enforced on CI off the bat (though in the future it could be), but at least having the tool available and easily usable would help to incrementally improve codebase style, and remove the need for formatting by hand. On all projects we've set up pre-commit with style formatting, it has significantly reduced noise and nitpicks in PR discussions. It makes for a better contributor experience, thus I am in favor. Black isn't perfect by any means, but never having to talk about formatting again makes it worth it! Stéfan I (and maybe some others) am missing some context. This somehow runs when I do "git commit"? Do I need to add anything to my git environment to pick up the hook? How do I turn it off if I find it prevents progress or breaks? Do we need to add anything to the documentation to warn developers about possible complications or errors? If it runs locally before committing, what does "enforce on CI" mean and how is that different from the current linting CI job? Matti ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
[Numpy-discussion] Re: Automatic formatters for only changed lines
Hi Matti, On Thu, May 5, 2022, at 22:08, Matti Picus wrote: > This somehow runs when I do "git commit"? Do I need to add anything to > my git environment to pick up the hook? How do I turn it off if I find > it prevents progress or breaks? Do we need to add anything to the > documentation to warn developers about possible complications or errors? The hook is optional, and is set up by running pre-commit (which itself is a pip-installable python package): pre-commit install If you want to turn it off, you simply uninstall it. `pre-commit` has a configuration file in which you set up all the linting-type checks you want to do (e.g. https://github.com/networkx/networkx/blob/main/.pre-commit-config.yaml). You could also run the linter manually: pre-commit run -a (on all files) pre-commit run filename And this is typically what happens in the CI (except that it will only run on modified files). > If it runs locally before committing, what does "enforce on CI" mean and > how is that different from the current linting CI job? You run the linter on CI same as now, to ensure that all style rules are met. But the hook gives a developer earlier warning, so they don't have to wait for CI runs to complete to know they need to fix things up. Stéfan ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
