search

nipreps / eddymotion

Open-source eddy-current and head-motion correction for dMRI.
https://nipreps.org/eddymotion
Apache License 2.0
13 stars 16 forks source link

Drop pre-commit config and hook? #146

Open oesteban opened 4 months ago

oesteban commented 4 months ago

I am not a fan of pre-commit - do we agree to remove it?

jhlegarreta commented 4 months ago

Not sure if I see the advantage of dropping it: please, correct me if I'm wrong, but when the non-compliant code is pushed, developers need to wait for the CI job, inspect it, fix any warning locally, then push again, wait for another cycle, inspect again whether there are warnings, etc. generating unnecessary noise. Not the best use of time, including that of the maintainers. In some cases, it may be days/weeks until the contributor addresses those changes, potentially delaying any other development that may depend on those changes. And people tend to add commits like STYLE: Fix PEP8 (this being the most descriptive commit subject) that IMO pollute the history and prevent focusing on relevant changes. All this assumes that people have done pip install -e .[dev] when contributing, which is not that much expensive (time, space) compared to a bare pip install -e .

But it is just an opinion of mine.

oesteban commented 4 months ago

I'm not against the pipeline, I'm concerned about the particular tool (or better, how the project is handled).

I guess I'll change my opinion when pre-commit supports its configuration within pyproject.toml or it is superseded by another project with different leadership.