Re: [PATCH 23/30] subtree: add comments and sanity checks
- From
Luke Shumaker <lukeshu@lukeshu.com>
- Date
- Apr 23, 2021, 23:58 UTC
- Message-ID
- <87y2d8a6zn.wl-lukeshu@lukeshu.com>
- In-Reply-To
- <CAPig+cQmTTdVKQptQY_tT_x4DCduRh=Mq6QZ1vUsc790-0yTQw@mail.gmail.com>
On Fri, 23 Apr 2021 14:58:34 -0600, Eric Sunshine wrote:
Show 11 quoted lines
>
> On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:
> > 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/ perhapsAck.
Show 19 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.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