Issue #353: Possible mercurial VCS hook location error
| Reported by: | Ross Thomas |
| State: | resolved |
| Created on: | 2020-01-24 22:15 |
| Updated on: | 2020-01-31 20:29 |
Description
In the Mercurial vcs handler the def for get_hook_location appears to have either a typo or a very confusing name.
def get_hook_location(self):
"""
returns absolute path to location where hooks are stored
"""
return os.path.join(self.path, '.hg', '.hgrc')
If is meant to be a distinct location from the standard repo rc file, then it may be better named ‘.hooks’ or some such.
If it is meant to be the standard repo hgrc then the '.' is a typo (unless I missed something wrt mercurial repos).
Sry, I don’t have enough insight into the design decisions for this to confidently make a PR.
Attachments
Comments
Comment by Thomas De Schampheleire, on 2020-01-25 20:08
Thanks for reporting this.
Luckily this function is not actually used: get_hook_location is implemented both for git and hg backends, but only used in a test for the git backend.
And for the git case: it is used to verify that function ScmModel.install_git_hooks works correctly, but install_git_hooks itself does not use get_hook_location but instead hardcodes the location.
So, I would argue to delete get_hook_location for both Mercurial and Git, and let the git test also hardcode the path. Alternatively, remove the hg implementation only, and let install_git_hooks also use get_hook_location.
@Mads Kiilerich What is your preference?
Comment by Mads Kiilerich, on 2020-01-25 23:01
Comment by Thomas De Schampheleire, on 2020-01-31 20:29
Pushed as 42ef4ea26efa, will be part of 0.6.0. Thanks again for reporting!