Currently the convention for repository hooks is to prefix their name with xvcs, such as xcvs or xsvn. For someone looking at the project for the first time, this does not give an indication into what the files or functions do.
I propose that the convention should be as follows:
* The hook files and information should be in a subdirectory called hooks within the specific VCS backend.
* The files should be called <vcs>_hook_<hook name>.php, i.e. svn_hook_pre_commit.php. There is the issue that the VCS hook names are usually pre-commit<code> rather than <code>pre_commit, so there would be an inconsistency between svn_hook_pre_commit.php for the svn hook called pre-commit.
* The function names should follow the same convention as the file names, so xsvn_init would become svn_hook_init.
This is a pretty minor issue, but could increase the ease of understanding by developers new to the project.
Comments
Comment #1
haxney commentedBy the way, I realize that this does not apply to versioncontrol.module so much as the individual backends. Since this affects all of the backends, as it is a policy change, so I figured it would be better to create one issue here until a final decision is made. At that point, we could create a series of individual issues for each backend.
Comment #2
jpetso commentedIn the light of this issue, is the hooks branch still subject to merge or should it be (again) rewritten to get better naming for the SVN backend from the start? (I don't care that much, so it's up to you how you feel about it. I'm going to merge that branch tomorrow, btw.) Mind that hook scripts in CVS are "natively" called trigger scripts.
(Background: The name "xcvs" stems from the time when I was not even yet involved with version control integration, and I inherited it from cvs.module. "xsvn" is something derived from that name, but in no way necessary. I have no preference what kind of naming to use.)
I would imagine that some VCSes (e.g. Git) might prefer hyphens instead of underscores (also, I like them better), is that an issue for inter-backend standards?
Comment #3
dwwi'm fine with killing "xcvs" ... that's just weirdness i inherited from kjartan.
i have a slight preference for _ in filenames for consistency (even if some vcs's call their hooks "pre-commit" not "pre_commit") especially for the "functions should match their filenames" point.
I'm slightly worried about calling the directory "hooks" (and embedding "hook" in the filenames) for fear of DX confusion with drupal hooks, etc. Not sure exactly what else to call them, however. If the files live in a directory called "hooks", I think putting "hook" in each filename is redundant.
Otherwise, I don't really care that much... The exact hooks are going to vary a bit from VCS to VCS. The most important thing is that each directory of these scripts contains a very clear README.txt or INSTALL.txt which explains what each script does and how to configure it in the specific VCS in question...
Comment #4
marvil07 commentedComment #5
marvil07 commentedI agree with dww about using README.txt in the hook directory to really document the files inside it.
I also do not want to make a confusion using hooks as the directory name, I guess a
vcs_hooksis a good enough name for now, since anyway drupal hooks are documented on the *.api.php file, so not a real confusion.I am adding this conclusion to the module documentation to make it clear for backend developers on both
7.x-1.xand6.x-2.x.Comment #6
senpai commented