Develop a git hook to detect "Topics missing from index" before commit or before push

[dmurdoch]

If I add a new function to my package but forget to add it to _pkgdown.yml, then the pkgdown build on Github will fail. It would be nice if one of the git hooks wouid warn me about this.

I'm not sure if the warning should be a pre-commit or pre-push warning.

Currently I don't have a pre-push hook, but I have a pre-commit hook that checks that README.md has been updated after README.Rmd was changed. I believe that hook was installed by usethis::use_readme_rmd(). I don't think anything in usethis installs a pre-push hook, so it would probably be easiest to do that, so you don't need to worry about merging the two hooks. I think usethis::use_pkgdown() would be the place to put it.

A test could be that if NAMESPACE is newer than docs/reference/index.html, it could suggest running pkgdown::build_site() or maybe pkgdown::build_reference(), which should update the index file.

[dmurdoch]

The test I suggested above is not sufficient. I just split a help page that was covering multiple functions into separate help pages, and that was enough to cause build_site() to fail. So maybe the test should be something like trying to run pkgdown::build_reference_index().

[jennybc]

Paging @hadley or @jayhesselberth

Can you comment on what it would take to do this? I haven't done any experiments, but have the vague sense that actually figuring this out would probably be too onerous for something we automatically put in a Git hook. I don't really mind learning this from a failed pkgdown run every now and then.

I will also say that the pre-commit hook we place with usethis::use_readme_rmd() has not been a universally happy experience. I still think that hook does more good than harm, but some people dislike it. I definitely don't want to place a new hook that is even more likely to generate pushback.

[jayhesselberth]

This has happened to me enough times that I would find it useful, but it would be equally annoying to the readme_rmd() hook (which I don't find all that annoying).

I think the best way would be to run pkgdown::check_pkgdown() in the hook. I can't think of a robust way using file modification times.

[dmurdoch]

I've been trying out a hook since around the time I posted this suggestion, and it is actually kind of irritating. This is the code I put in my pre-commit hook:

if [[ NAMESPACE -nt docs/reference/index.html ]]; then
  echo -e "pkgdown index may be out of date; please run pkgdown::build_site()\n$MSG"
  exit 1
fi

I think I actually run pkgdown::build_reference_index() instead of building the whole site as my warning suggests; it will fail faster. It might be less irritating as a pre-push hook, or just unconditionally running build_reference_index() every time.

[hadley]

I have a general aversion to git hooks, so I don't have anything to add here.

[jennybc]

I think that such a git hook, should it exist, should morally be "owned" by pkgdown. The necessary script could be hosted somewhere within pkgdown. And if it proves to be really useful and we guess that almost all pkgdown users and sites will want it, then we can revisit usethis placing this git hook as part of use_pkgdown(). usethis has really nice ways to do that. But I don't think we have enough cause for usethis to start doing this now.

@jayhesselberth Do you want me to transfer this issue to pkgdown for further discussion? The alternative is me just closing it.

[jayhesselberth]

@jennybc yes, please transfer and we'll see if it gets any traction there.