Re: [PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved

From: Luke Diamand <luke@diamand.org>
Date: Oct 12, 2018, 18:19 UTC
Message-ID: <CAE5ih7_sT8aP7n6h0moEj=6JjRAGJ+AXUCDkTFoJmnbE_Nh1mA@mail.gmail.com>
In-Reply-To: <xmqq7einb7ob.fsf@gitster-ct.c.googlers.com>

On Fri, 12 Oct 2018 at 14:45, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

>
> Luke Diamand <luke@diamand.org> writes:
>
> > The branch detection code looks for branches under refs/remotes/p4/...
> > and can end up getting confused if there are unshelved changes in
> > there as well. This happens in the function p4BranchesInGit().
> >
> > Instead, put the unshelved changes into refs/remotes/p4-unshelved/<N>.
>
> I am not a p4 user (and not a git-p4 user), so it is a bit hard for
> me to assess if this is a backward incompatibile change and if so
> how serious potential breakage to existing users would be.

I don't think it's a particularly serious breakage - it reports the branch it unshelves to, so it should be fairly obvious.

However, maybe it would make sense to pull this into a separate commit to make it more obvious? I should have thought of that before submitting.

Show 13 quoted lines

>
> >
> > -If the target branch in refs/remotes/p4/unshelved already exists, the old one will
> > +If the target branch in refs/remotes/p4-unshelved already exists, the old one will
> >  be renamed.
> >
> >  ----
> >  $ git p4 sync
> >  $ git p4 unshelve 12345
> > -$ git show refs/remotes/p4/unshelved/12345
> > +$ git show p4/unshelved/12345
>
> Isn't this "p4-unshelved/12345" now?

Yes, I think another reason to pull into a separate commit.

Luke