Re: [PATCH 1/1] git-p4: add unshelve command

From: Luke Diamand <luke@diamand.org>
Date: Feb 22, 2018, 22:28 UTC
Message-ID: <CAE5ih79E1Z8h+hAgr8zCw7=Dtec6sxT7Xf4cWn8zOLNs=mY1jg@mail.gmail.com>
In-Reply-To: <CAKYtbVYe5hHDXf=nQzt6r9N20CrTgbEMbBS0JBBXQLpz+BVu1A@mail.gmail.com>

On 22 February 2018 at 21:39, Miguel Torroja <miguel.torroja@gmail.com> wrote:

Show 11 quoted lines

> Hi Luke,
>
> I really like the idea of creating a branch based on a shelved CL (We
> particularly use shelves all the time), I tested your change and I
> have some comments.
>
>  - I have some concerns about having the same "[git-p4...change =
> .....]" as if it were a real submitted CL.
>     One use case I foresee of the new implementation could be to
> cherry-pick that change on another branch (or current branch) prior to
> a git p4 submit.

OK, I think we could just not add that in the case of an unshelved commit.

>
>  - I see that the new p4/unshelve... branch is based on the tip of
> p4/master by default. what if we set the default to the current HEAD?

There's a "--origin" option you can use to set it to whatever you want.

I started out with HEAD as the default, but then found that to get a sensible diff you have to both sync and rebase, which can be quite annoying.

In my case, in my early testing, I ended up with a git commit which included both the creation of a file, and a subsequent change, even though I had only unshelved the subsequent change. That was because HEAD didn't include the file creation change (but p4/master _did_).

Show 5 quoted lines

>
>  - Shelved CLs can be updated and you might have to run "git p4
> unshelve" a second time to get the latest updates. if we call it a
> second time it fails as it's trying to update p4/unshelve... rather
> than discarding previous one and creating a new one.

OK, that should also be fixable.

>
>
> Thanks,

Thanks for the feedback, very useful! I'll reroll. Luke