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

Feature Request: use_pre_commit_config() #849

Closed
njtierney opened this issue Jul 22, 2019 · 6 comments
Closed

Feature Request: use_pre_commit_config() #849

njtierney opened this issue Jul 22, 2019 · 6 comments

Comments

@njtierney
Copy link
Contributor

I've recently enjoyed using @lorenzwalthert 's pre-commit-hooks package - I'm wondering if there might be some scope to include this in usethis?

This would/could:

  • install pre-commit-hooks via homebrew (or similar?)
  • Add ^\.pre-commit-config\.yaml$ to .Rbuildignore
  • run pre-commit install to install the pre-commit hooks
use_pre_commit_config()
# magic things happen
@lorenzwalthert
Copy link
Contributor

I think there is a conflict with current infrastructure in usethis like usethis::use_git_hook() and the README hook. At least pre-commit complained when I installed it and there were non-pre-commit.com hooks in the repo. I think we need to investigate this in case the maintainers of usethis consider adding such a functionality. Also, would you suggest to add specific hooks (like the ones in https://github.com/lorenzwalthert/pre-commit-hooks) or simply getting the stage ready for people to create and populate their .pre-commit-config.yaml themselves?

Side note: I am not sure I'd call my repo a package. I try to give it this structure, but it's not there 100%, but I think it does not matter that much if it one because it is basically bash scripts and pre-commit will handle the installation process.

@jennybc
Copy link
Member

jennybc commented Jul 23, 2019

I'm not eager for usethis to get deeper into the commit hook business. The one we do add (about README.[R]md stuff) is already a source of some complaints / friction. I think doing this well (hook management) is outside the scope of usethis. Empirically, I've concluded you also can't lean too much on such hooks because they don't travel with the repo and libgit2 doesn't support hooks, so clients that rely on libgit2 effectively don't "see" them.

@jennybc jennybc closed this as completed Jul 23, 2019
@njtierney
Copy link
Contributor Author

njtierney commented Jul 24, 2019 via email

@lorenzwalthert
Copy link
Contributor

lorenzwalthert commented Jul 24, 2019

Maybe we could implement something similar to what you proposed in lorenzwalthert/pre-commit-hooks then.

Side note: I am not sure I understand you correctly as far as the traveling of hooks goes. I think it is a key feature of pre-commit.com that they are the same for every user working in the same repo because the hooks are defined in a central place (a git repo like the one referenced above) and a user only needs a .pre-commit-config.yaml in his repo and pre-commit.com will administer the hooks in ./git/hooks.

@jennybc
Copy link
Member

jennybc commented Jul 24, 2019

This is the sense in which I mean git hooks "don't travel with the repo":

However, those scripts are not propagated when people do a clone and they are not version controlled.

https://stackoverflow.com/questions/427207/can-git-hook-scripts-be-managed-along-with-the-repository

or

By default hooks are stored in .git/hooks outside of the working tree and are thus not shared between users of the repository

https://www.darrenlester.com/blog/including-hooks-in-a-git-repository

@lorenzwalthert
Copy link
Contributor

lorenzwalthert commented Jul 24, 2019

Yes, in my understanding, these shortcomings are only completely resolved when pre-commit install is run after cloning and hence the content of the hooks is in ambiguously determined via the (git) tag field specified in the version controlled .pre-commit-config.yaml.

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

3 participants