Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pre-commit clarification #368

Open
sostrades-pjbarjhoux opened this issue Dec 21, 2023 · 1 comment
Open

pre-commit clarification #368

sostrades-pjbarjhoux opened this issue Dec 21, 2023 · 1 comment

Comments

@sostrades-pjbarjhoux
Copy link

In this pull-request, I see that there is a pre-commit step where multiple checks are done, including these checks on markdown files :

precommit

My questions are :

  • is this check blocking to merge the PRs ?
  • the checks are performed on ALL the files of the repositories, not only the ones that are commited. Can we change this?
  • from what I understand (from this PR), the pre-commit rules seem to be automatically changed by a bot. If this is the case, is it the expected behaviour ? it could randomly impact the repositories and code deliveries. From my point of view, these actions should be planned and communicated so that we prepare the repositories before, to be compliant with the new rules?
@MichaelTiemannOSC
Copy link
Contributor

Commenting from my perspective as a committer/maintainer of another repo that has the same rules applied.

pre-commit does not block the merging of PRs, but the pre-commit bot can modifying non-conforming changes before you can stop it. I see this most often with black reformatting my code. In the repo in which I work, mypy and flake8 can fail and I can still commit code (and the code isn't changed). But because of reformatting from pre-commit, I need to fetch and pull those changes before I continue on my merry way--a big change to my often solo work.

An additional wrinkle is that pyproject.toml is really part of a larger pdm/tox build system. Manual changes to pyproject.toml can and does get overwritten by pdm. I'm slowly wrapping my head around this and accepting it, though it disturbed me at first.

On the question of pre-commits scope, did you mean "directories" instead of "repos"?

I do believe that @ModeSevenIndustrialSolutions is working on docs of the best practices being implemented across all the repos. I've been at the bleeding edge of that as a user. There have been moments of frustration, but overall I'm very happy with how much and how well things have progressed in terms of making my code more consistent with the various PEPs that I don't even know about.

Please don't take this as me trying to shut down any critique of what's been happening. I'm just chiming in from my perspective, and all should feel free to elaborate on what they do and don't like, and how to make things better for the project.

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

No branches or pull requests

2 participants