Possible mercurial VCS hook location error

Issue #353 resolved
Ross Thomas created an issue

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.

Comments (3)

  1. Thomas De Schampheleire

    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?

  2. Log in to comment