Re: [PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
On Fri, 12 Oct 2018 at 19:19, Luke Diamand wrote: > > On Fri, 12 Oct 2018 at 14:45, Junio C Hamano wrote: > > > > Luke Diamand 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/. > > > > 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. > > > > > > > > > -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. D'oh. It's already in a separate commit. I'll re-roll fixing that documentation. I think it will be fine to change the branch that the unshelving happens into - I think it's very unlikely anyone is basing some automated scripts on this, because of the way that unshelving is used anyway. Luke
Re: [PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
On Fri, 12 Oct 2018 at 14:45, Junio C Hamano wrote: > > Luke Diamand 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/. > > 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. > > > > > -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 >
Re: [PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
Luke Diamand 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/. 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. > > -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?
