README commit hook prevents commit if only .md file changed

[dirkschumacher]

When using an README.Rmdwhere the output of chunks can change without changing the code in the Rmd file, the commit hook in usethis::use_readme_rmd prevents the resulting README.md file from being commited to git.

I sometimes add coverage results into the readme during initial development of a package. When knitting the README.Rmd, often only the README.md file changes.

Below is a simple example where the README.Rmd file outputs a random number. The commit hook prevents adding a new README.md without making a change to README.Rmd.

git init

Rscript -e 'usethis::use_readme_rmd()'

echo '---
output: github_document
---
# my readme
```{r}
runif(1)
```' > README.Rmd

Rscript -e 'knitr::knit("README.Rmd")'

git add README.Rmd README.md
git commit -m "initial commit"

Rscript -e 'knitr::knit("README.Rmd")'

git add README.md
git commit -m "this commit does not work"

[jennybc]

Yeah, I have experienced this too. I then follow the directions about how to commit anyway (which forces me to commit from the command line).

I don't see a simple way to prevent this regrettable behaviour, while also catching the (more likely? worse?) situation of someone editing their README.md by hand, even though it's generated by README.Rmd.

Do you have any ideas re: resolution? Would you prefer to have no pre-commit hook at all?

[dirkschumacher]

The question is how often the README.md file gets changed without the .Rmd file. I could imagine it is often the other way round: README.Rmd file gets changed, but the user forgot to knit a new .md file.
One solution could be to make the rule less conservative: "if README.Rmd staged, then README.md must be staged" instead of "README.Rmd staged <=> README.md staged".

On the other hand, git directly presents a solution on how to commit anyways. So maybe it is fine to leave it as conservative as it is.

[jennybc]

I think we will leave it for now, as I think it's the best compromise. If you end up modifying the hook as described above for yourself, please comment here, so we can store/link it for posterity as an alternative to removing the hook.

[petermeissnerbot]

I think that in general convenience and safeguard functionality should not prevent me from doing stuff - it should help if helping does not harm.

This functionality is harmful (I use automatic information and statistics in README.Rmd so I re-knit regularly, now I cannot because usethis tinkered with my Git-repo without actually telling me beforehand; example: https://github.com/petermeissner/ical/blob/master/README.Rmd).

I think this is a very bad tradeoff - I feel patronized and a little pissed off at the moment - and I will not do as usethis suggests and work around the new problem by going to the terminal (where is the convenience in that?) but simply either re-hack whatever usethis did (if possible) or stop using usethis (which is unfortunate because it offers some very-very-very nice tooling otherwise).

So please consider re-opening this issue and give back users freedom to automate stuff in their scripting language :-) - maybe a short option asking the user when usethis::use_readme_rmd() is called to opt into this check would be a better solution.

---

For future reference: the problem can be solved by

• going into the terminal
• switching to your repo
• going to .git/hooks/ and moving / deleting / commenting out the following file: pre-commit

[Robinlovelace]

Adding a pre-commit hook is quite an invasive change that could end up being more time consuming and frustrating for users, especially beginners new to git, than is justified by the potential benefits. It can be useful to be asked to rebuild the README.md if you make a change to README.Rmd and forget to do so, but I'm not sure the +s outweigh the -s.

The mantra of 'do no harm' is important in life and in software and I suspect some people have been confused by this. Another way of looking at this is that it reminds me of a common critique of Microsoft products that it is 'too clever', and sometimes does things that are unhelpful. For an interesting, and quite funny, thread on this, see here!

Good news: I'm sure there are various solutions out there that could help maintain the +s of the current set-up without the -s. This issue could be be a place to help identify the most appropriate, if assuming the current behaviour is not set in stone. An idea I thought of just now after following 'future reference' section at the bottom of the previous message:

• When running usethis::use_readme_rmd() for the first time in interactive mode ask the user if they'd like the pre-commit hook added (that could have the added advantage of emphasising the fact that it is there, e.g. in a message like Would you like a pre-commit hook to be added to ensure that the README.md file is more recent than README.Rmd? If Yes, a file located at .git/hooks/pre-commit will be created. You can move or modify this file at a later date.

There may be negatives of this suggestion I've not considered and happy to talk about them. Overall very pleased with the usethis package, it's saved me lots of time and only cost me time on a few occassions (most of them have involved this issue, hence piping up). Thanks for the package + discussion @jennybc, happy to contribute further ideas and potentially a PR if that would be welcome.

P.s. this is what I ran just now to be able to push an updated version of README.Rmd, in case it's of use/interest to others who're asking the same question, how do I commit my updated README,Rmd file when README.md has not changed:

 mv .git/hooks/pre-commit /tmp/
 git commit -am 'Update readme.Rmd'

[jennybc]

Everywhere I work (shell, RStudio,and GitKraken), here's what happens when I trigger this hook:

README.Rmd and README.md should be both staged
use 'git commit --no-verify' to override this check

80% of the time, I realize I've edited the wrong file or forgotten to re-render and am thankful. The other 20% I do exactly as the message says:

git commit --no-verify -m "my commit message"

Do you not get these instructions? What interface to git do you use? Also, as you've clearly sorted out, you can delete this hook if it somehow does you more harm than good:

rm .git/hooks/pre-commit

[Robinlovelace]

I do get those instructions and that's what I did before removing the pre-commit hook. However there are some projects where it's useful to not to have to use that solution. That could happen if README.Rmd is a manuscript that compiles to .pdf, if minor changes are frequently made to the .md document but not the .Rmd as @dirkschumacher found or in other cases, such as documented by @petermeissnerbot or when a user runs usethis::use_readme_rmd() by accident or later changes their mind. Another issue not covered by the message is that it tells you what to do but not how it works which can be more important: users should know when bits of their project are being changed.

To save adding the --no-verify bit before every commit I've removed the pre-commit hook as outlined above. I think users, especially those new to git, would benefit from knowing that they can do that. At the very least a message with that instruction could appear when usethis::use_readme_rmd() runs e.g.

Adding a pre-commit hook. This can be removed with file.remove(".git/hooks/pre-commit")

Would help users who didn't know that the change isn't necessarily permanent and what is going on under the hood, which has co-benefits from a learning perspective. I'm a long time R and git user and I didn't know so suspect that's most people. A good compromise?

[Robinlovelace]

P.s. to answer your other question, I'm running R/RStudio on Ubuntu with git 2.17.1.

[jennybc]

I think it sounds like you should remove the hook in that project, since it's a net negative for you.

> use_readme_rmd()
✔ Setting active project to '/Users/jenny/tmp/githooktest'
✔ Writing 'README.Rmd'
✔ Adding '^README\\.Rmd$' to '.Rbuildignore'
● Modify 'README.Rmd'
✔ Writing '.git/hooks/pre-commit'

The pre-commit hook is messaged at creation. In theory we could add more detail. But I have found that this is no panacea because people, frankly, often don't really read messages, especially not long ones! Even if they do, they won't retain this information over the weeks or months between creating the project and the first time they bump into this.

If we can keep it short (on one line) and parallel with the other messages above, I certainly am content to modify this message.

I suspect this is one of those things you sort of have to bang your shins on. We could remove the git hook and then, based on my experience, I would just bang my shins on changing README.Rmd and forgetting to re-render.

[Robinlovelace]

Fair point that the message does tell you where it is at creation time. Yet another option would be to provide an arguement, set_precommit_hook = TRUE in the function but, as you indicate, that could add further complexity and create yet another place for people to bang their shins against (good analogy btw, especially for a cycling afficionado who bashed their shins many a-time while learning). In summary (to provide another transport analogy) we've kicked the tires of use_readme_rmd() and it's stood-up well from its maintainer's perspect, who's provided reasons for the way it is. Another - of changing things is that inevitably makes people bash their shins as they adapt!