Re: [PATCH 23/30] subtree: add comments and sanity checks

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 23, 2021, 20:58 UTC
Message-ID: <CAPig+cQmTTdVKQptQY_tT_x4DCduRh=Mq6QZ1vUsc790-0yTQw@mail.gmail.com>
In-Reply-To: <20210423194230.1388945-24-lukeshu@lukeshu.com>

On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:

Show 7 quoted lines

> Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> ---
> 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

Show 14 quoted lines

>  # 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.