Repository: innolitics/rdm (language: python)

Issue:
Alter and/or document hook logic
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]+`)](https://github.com/innolitics/rdm/blob/e2ab8b02d42b003a7064d5c460457cff3bff0f8e/rdm/hook_files/prepare-commit-msg#L8).

## 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:

```diff
- 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](https://regexr.com/6j35l).


Discussion / hints:
@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.
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.
^ 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.
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.
I'm up for taking it on - sounds like fun! I'll probably get to this tomorrow or over the weekend.

Fix patch:
diff --git a/README.md b/README.md
index cc8f2b9..c685990 100644
--- a/README.md
+++ b/README.md
@@ -19,8 +19,13 @@ RDM is especially well-suited for early-stage software-only medical devices that
 
 ## Quick Start
 
-```
+```sh
 pip install rdm[github]
+
+# Optional: Install git hooks
+rdm hooks
+
+# Scaffold initial template files and run
 rdm init
 cd regulatory
 make
@@ -71,6 +76,7 @@ The best companies follow the regulations with a degree of faith that these regu
 RDM is designed to be used within a typical software development workflow.  When a new project is started, developers
 
 1. Install RDM using `pip install rdm`
+    A) Optional: Install [RDM's git hooks](#git-hooks) using `rdm hooks` 
 2. Generate a set of markdown templates, which are stored in the git repository, using `rdm init`
 3. Edit configuration variables in the generated files
 4. Write _software requirements_ and store them in the git repository
@@ -162,7 +168,7 @@ The `rdm render` subcommand provides a few extra filters to the Jinja context:
 
 ### Extensions
 
-We also support [extensions](http://jinja.pocoo.org/docs/2.10/extensions/). Extensions are set using the `md_extensions` configuration paramater in `config.yml`. See the Markdown Extensions section for details about available markdown extensions.
+We also support [extensions](http://jinja.pocoo.org/docs/2.10/extensions/). Extensions are set using the `md_extensions` configuration parameter in `config.yml`. See the Markdown Extensions section for details about available markdown extensions.
 
 ## YAML Front Matter
 
@@ -319,6 +325,20 @@ To provide a custom checklist, use a file path for the first argument.
 
 The checklist format is described in detail [here](./docs/checklist-format.md).
 
+## Git Hooks
+
+Most git hosting platforms, e.g., GitHub, GitLab, and Bitbucket, allow you to trace git commits back to their originating issues using specially formatted comments. For example, if a commit is related to issue #123 you might include the text "Issue #123" in the git commit message. Maintaining this traceability helps RDM auto generate certain regulatory documents, but these links to every commit becomes tedious.
+
+The `rdm hooks` command installs git hooks that will automatically add issue (or, in regulatory speak, "change request") numbers to your git commit messages, based on your branch name.
+
+> When using this feature, name your branches in the style of `${hyphen-separated-issue-numbers}-rest-of-branch-name`.
+>
+> Example: `12-13-fixing-issues`
+
+Grabbing the issue number from the branch name works well since most of the time a branch will involve working on a particular issue. You can always edit the auto-generated messages on a per-commit basis if needed.
+
+If you manually edit one of the provided hooks and want to revert your changes or RDM has been updated and you want to make sure the newest hooks are used, you can re-run this command at any time.
+
 ## Contrib
 
 The [contrib folder](https://github.com/innolitics/rdm/tree/main/contrib) includes several scripts and files which may be useful to you. Each is described in some detail in the [README](https://github.com/innolitics/rdm/tree/package-format-script/contrib#readme) within the folder.
diff --git a/rdm/hook_files/prepare-commit-msg b/rdm/hook_files/prepare-commit-msg
index 3819962..8d3868f 100644
--- a/rdm/hook_files/prepare-commit-msg
+++ b/rdm/hook_files/prepare-commit-msg
@@ -5,7 +5,7 @@
 #
 append_feature_number_to_commit_message() {
     BRANCH=$(git branch | grep '*' | sed 's/* //')
-    ISSUES=$(echo "$BRANCH" | grep -o -E '[0-9]+')
+    ISSUES=$(echo "$BRANCH" | grep -o -E '^[0-9,-]+' | grep -o -E '[0-9]+')
     for ISSUE_NUMBER in ${ISSUES[@]}
     do
         echo -e "$(cat "$1")\n\nIssue #$ISSUE_NUMBER" > "$1"


Test patch:
diff --git a/tests/hook_test.py b/tests/hook_test.py
index 5c4bebf..293f300 100644
--- a/tests/hook_test.py
+++ b/tests/hook_test.py
@@ -80,10 +80,10 @@ def test_multiple_issues(tmp_repo):
     assert show_commit_message() == "Fix some issue\n\nIssue #10\n\nIssue #11\n\n"
 
 
-def test_text_before_issue(tmp_repo):
-    prepare_branch(tmp_repo, 'fix-10-sample-issue')
+def test_mixed_non_issue_numbers(tmp_repo):
+    prepare_branch(tmp_repo, '42-85-implement-base-64-on-aws-s3')
     tmp_repo.git.commit('-m', 'Fix some issue')
-    assert show_commit_message() == "Fix some issue\n\nIssue #10\n\n"
+    assert show_commit_message() == "Fix some issue\n\nIssue #42\n\nIssue #85\n\n"
 
 
 def test_no_issue_number(tmp_repo):