On Fri, 23 Apr 2021 14:58:34 -0600,
Eric Sunshine wrote:

On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker [off-list ref] wrote:

quoted

Signed-off-by: Luke Shumaker <redacted>
---

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh

@@ -248,17 +263,22 @@ rev_exists () {
+# Usage: try_remove_previous REV
+#
 # if a commit doesn't have a parent, this might not work.  But we only want

s/if/If/ perhaps

Ack.

quoted

 # to remove the parent from the rev-list, and since it doesn't exist, it won't
 # be there anyway, so do nothing in that case.

@@ -302,10 +322,12 @@ find_latest_squash () {
+# Usage: find_existing_splits DIR REV
 find_existing_splits () {
+       assert test $# = 2
        debug "Looking for prior splits..."
        dir="$1"
-       revs="$2"
+       rev="$2"

@@ -314,7 +336,7 @@ find_existing_splits () {
        git log --grep="$grep_format" \
-               --no-show-signature --pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |
+               --no-show-signature --pretty=format:'START %H%n%s%n%n%b%nEND%n' "$rev" |

The caller of this function is passing in "$revs". Did you make this
semantic change because the caller's `revs` is guaranteed to be a
single rev? In any case, this change may deserve mention in the commit
message so readers don't have to wonder about it.

No it's not?  The only caller is

	unrevs="$(find_existing_splits "$dir" "$rev")" || exit $?

But yeah, this would be a good thing for me to call out in the commit
message.

-- 
Happy hacking,
~ Luke Shumaker