Alter and/or document hook logic

[joshuatz]

I've started using the git commit hooks provided by RDM and ran into an unexpected outcome today. I'm not sure I would classify it as a bug, but I think it might not match assumptions made by users and should probably either be updated or perhaps even just documented with a warning somewhere.

(I also noticed that rdm hooks is documented within init_files, but not in the main README. I've already found the hook useful after just a few days of use, so it seems to me worth highlighting in the main README, perhaps in the Quick Start section?)

Issue

Given a branch name of 245-fix-uploads-to-s3, the commit message will be pre-populated with:

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch 245-fix-uploads-to-s3
# Changes to be committed:
#   ...
#

Issue #245

Issue #3

I certainly didn't intend for Issue #3 to get linked, nor do I think most users would.

I suppose this probably doesn't happen that often, but could still see it happening more than once, with branch names like __-migrate-to-python-v3, __-implement-s3, __-fix-base64-usage, etc.

Cause

Looks like the pattern for extracting the issue is pretty broad ([0-9]+).

Proposed solution

Although users can easily edit the script themselves, it might be nice to either call this out somewhere in the docs, or refine the regex a little to provide a different default.

For example, although not very nice to look at (this was a quick pass; could probably be refined more), this avoids the above issue, as well as some other edge-cases:

- ISSUES=$(echo "$BRANCH" | grep -o -E '[0-9]+')
+ ISSUES=$(echo "$BRANCH" | grep -o -E '^([0-9]+)|[^A-z.0-9]([0-9]+)[^A-z.0-9]*?' | grep -o -E '[0-9]+')

Test cases here.

[johndgiese]

@joshuatz this is definitely an annoying shortcoming of RDM's hooks; I've run into it a bunch of times.

@russellkan and @joshuatz , what do you guys think of only creating issues for dash-separated numbers at the start of the branch. Thus:

1-asdf --> 1 1-2asdf --> 1 1-2a-asdf --> 1 1-2-asdf --> (1, 2) 1-2-asdf-4 --> (1, 2) 1-2-3-asdf --> (1, 2, 3)

Also, I think describing this in the README is a good idea.

[russellkan]

I think that makes sense. It catches pretty much any case of numerals that aren't intended to be part of the issue listing, and adding a description to the README would definitely be helpful for users.

[joshuatz]

^ Agreed. Plus, it seems like enforcing a standard naming pattern for branches is a best practice anyways, and as a user I am usually happy to give up a little flexibility in exchange for more predicable behavior.

[johndgiese]

Great, if either of you feel like implementing this, that would be great! There is an internal project in Harvest for RDM. I added both of you to it.

[joshuatz]

I'm up for taking it on - sounds like fun! I'll probably get to this tomorrow or over the weekend.