Add pre-commit configuration file

[willschlitzer]

To prevent the annoyance of realizing I forgot to run black after I have pushed a commit, I think it would be good to add a pre-commit hook configuration file to automatically run black, flake8, and other formatting packages.

Are you willing to help implement and maintain this feature? Yes

[weiji14]

We'll need to discuss whether to add this. I recognize the value of using pre-commit, but I find that it is another layer of friction for new contributors to set up locally. Black is quite fast to run, but pylint can be very slow and I'd hate to run it every commit! We could also look at streamlining/improving the /format slash command if that's an issue.

[willschlitzer]

We'll need to discuss whether to add this. I recognize the value of using pre-commit, but I find that it is another layer of friction for new contributors to set up locally. Black is quite fast to run, but pylint can be very slow and I'd hate to run it every commit! We could also look at streamlining/improving the /format slash command if that's an issue.

I think of it as an optional add; contributors can choose to use pre-commit and already have a configuration file already to go. But I agree that making its usage mandatory is another barrier for entry for new contributors.

[maxrjones]

We'll need to discuss whether to add this. I recognize the value of using pre-commit, but I find that it is another layer of friction for new contributors to set up locally. Black is quite fast to run, but pylint can be very slow and I'd hate to run it every commit! We could also look at streamlining/improving the /format slash command if that's an issue.

Does the /format slash command not work for forks? I haven't had much success with using it lately.

[willschlitzer]

I tried this out on my local machine and it wasn't as user friendly as I hoped. I'm sure it can be configured to work alongside our make commands, but I'm going to close this issue since there doesn't seem to be much interest in it.

[maxrjones]

I've started using pre-commit while working on some other repositories and find it really nice. If there turns out to be interest from others in this tool, I could help set it up for the repository. I think it may be less friction for new contributors (install pre-commit and run pre-commit install once) than remembering the make commands given that it's getting to be fairly common.

[weiji14]

Is it worth trying https://pre-commit.ci?

[maxrjones]

Is it worth trying https://pre-commit.ci/?

Yes, I think we could try this as an alternative to the format slash command and probably the style checks workflow.

[seisman]

I recognize the value of using pre-commit, but I find that it is another layer of friction for new contributors to set up locally. Black is quite fast to run, but pylint can be very slow and I'd hate to run it every commit!

I agree.

Is it worth trying https://pre-commit.ci/?

Yes, I think we could try this as an alternative to the format slash command and probably the style checks workflow.

The pre-commit.ci service commits changes directly to the branch that triggers the CI, which means that the PR authors must update their local branch (e.g., git pull) before pushing more changes to remote (i.e., git push), otherwise they will see errors like:

error: failed to push some refs to 'git@github.com:xxx/xxx.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

which are not friendly for git newbies.

In short, I'm inclined to not use pre-commit and close the issue.

[maxrjones]

The pre-commit.ci service commits changes directly to the branch that triggers the CI, which means that the PR authors must update their local branch (e.g., git pull) before pushing more changes to remote (i.e., git push), otherwise they will see errors like:

This is configurable and can be disabled so that the CI service doesn't push changes, but I'll close this since I was the one encouraging using pre-commit and haven't been working on PyGMT lately.