Fixing remote branch creation (only want master in github).

Fixing remote branch creation (only want master in github).

If you create a branch 'a', then branch off that to 'b', then running 'git cl push' from 'b' will create a branch 'b' in origin instead of just pushing to master.

This change to the PRESUBMIT would set the upstream to master before pushing.

R=jackhou@chromium.org, tapted@chromium.org

Committed: https://github.com/tapted/bleeding_edge/commit/1c9f28d

[tapted, 2014-01-22 05:54:24]

+jackhou for review too, since I think you put this together initially

For a bugfix, the CL description needs to describe the current problem, and how
this change fixes it (not just that it "does" fix things).

E.g.

Currently if you git cl push on a blah with blah it will result in blah, which
will cause blah.

This change will instead do blah so that when blah happens it will use the
correct blah when running blah.

https://codereview.appspot.com/49670045/diff/1/dart/sdk/lib/_internal/pub/PRE...
File dart/sdk/lib/_internal/pub/PRESUBMIT.py (right):

https://codereview.appspot.com/49670045/diff/1/dart/sdk/lib/_internal/pub/PRE...
dart/sdk/lib/_internal/pub/PRESUBMIT.py:88: ['git', 'config',
'branch.master.remote', 'origin'])
I think `git cl push` will still grab the destination from branch.%s.remote no?

[pajamallama, 2014-01-24 03:45:48]

Made a proper go at this one.

[jackhou, 2014-01-24 03:50:21]

If you set the upstream to master, won't the patch contain all changes between
master and the current branch? (i.e. it'll be 'b' vs master, not 'b' vs 'a').

[pajamallama, 2014-01-24 04:02:55]

On 2014/01/24 03:50:21, jackhou wrote:
> If you set the upstream to master, won't the patch contain all changes between
> master and the current branch? (i.e. it'll be 'b' vs master, not 'b' vs 'a').

No I don't believe so. Say 'a' & 'b' are ready to push. I've done in the past:

git checkout a
git cl push

git checkout master
git pull --rebase

git checkout b
git branch --set-upstream-to=master
git cl push

[jackhou, 2014-01-24 05:25:03]

On 2014/01/24 04:02:55, pajamallama wrote:
> On 2014/01/24 03:50:21, jackhou wrote:
> > If you set the upstream to master, won't the patch contain all changes
between
> > master and the current branch? (i.e. it'll be 'b' vs master, not 'b' vs
'a').
> 
> No I don't believe so. Say 'a' & 'b' are ready to push. I've done in the past:
> 
> git checkout a
> git cl push
> 
> git checkout master
> git pull --rebase
> 
> git checkout b
> git branch --set-upstream-to=master
> git cl push

In the above example, there's no difference between master and 'a'. What would
happen if:

git checkout a
<commit some changes>

git checkout -b b
<commit some changes>
git cl push

I think we'd want to push the changes in 'b' relative to its upstream ('a').
Or maybe a better solution is that we don't support this use case and tell the
user to rebase.

[pajamallama, 2014-01-28 03:57:31]

That particular use-case seems like it would be almost guaranteed to create
confusing conflicts.

These days whenever I hit a strange presubmit error (the usual one being
that it says the local
patch is different to the lgtm'd one) I resort to

    git branch --set-upstream-to=origin/master;
    git cl upload --bypass-hooks

.. otherwise I end up bugging tapted too much to fix things :/

P.S I usually try --no-find-copies before --bypass-hooks since git finding
random copies of my
file frequently confuses rietvelt.


On Fri, Jan 24, 2014 at 4:25 PM, <jackhou@chromium.org> wrote:

> On 2014/01/24 04:02:55, pajamallama wrote:
>
>> On 2014/01/24 03:50:21, jackhou wrote:
>> > If you set the upstream to master, won't the patch contain all
>>
> changes between
>
>> > master and the current branch? (i.e. it'll be 'b' vs master, not 'b'
>>
> vs 'a').
>
>  No I don't believe so. Say 'a' & 'b' are ready to push. I've done in
>>
> the past:
>
>  git checkout a
>> git cl push
>>
>
>  git checkout master
>> git pull --rebase
>>
>
>  git checkout b
>> git branch --set-upstream-to=master
>> git cl push
>>
>
> In the above example, there's no difference between master and 'a'. What
> would happen if:
>
> git checkout a
> <commit some changes>
>
> git checkout -b b
> <commit some changes>
> git cl push
>
> I think we'd want to push the changes in 'b' relative to its upstream
> ('a').
> Or maybe a better solution is that we don't support this use case and
> tell the user to rebase.
>
> https://codereview.appspot.com/49670045/
>

[jackhou, 2014-01-28 04:25:31]

I guess the check that the uploaded patch matches the local patch will prevent
the error I outlined.

This CL seems fine then, but see what tapted thinks.

lgtm

[tapted, 2014-01-28 05:37:53]

lgtm with one suggestion

https://codereview.appspot.com/49670045/diff/20001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/PRESUBMIT.py (right):

https://codereview.appspot.com/49670045/diff/20001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/PRESUBMIT.py:96: if not upstream.endswith('master'):
Maybe endswith ('.master') (e.g. in case someone makes a branch called
"my-copy-of-master"). Maybe optionally `== 'master'` if that's legal too

[pajamallama, 2014-01-28 06:20:19]

Committed patchset #4 manually as r6f30f37 (presubmit successful).

[pajamallama, 2014-01-28 06:39:04]

Committed patchset #5 manually as r1c9f28d (presubmit successful).

[pajamallama, 2014-01-28 06:40:41]

Okay guys. Made some relatively big changes and pushed them.... soz! I wanted to
actually test it worked, and now it does.

https://codereview.appspot.com/49670045/diff/20001/dart/sdk/lib/_internal/pub...
File dart/sdk/lib/_internal/pub/PRESUBMIT.py (right):

https://codereview.appspot.com/49670045/diff/20001/dart/sdk/lib/_internal/pub...
dart/sdk/lib/_internal/pub/PRESUBMIT.py:96: if not upstream.endswith('master'):
On 2014/01/28 05:37:53, tapted wrote:
> Maybe endswith ('.master') (e.g. in case someone makes a branch called
> "my-copy-of-master"). Maybe optionally `== 'master'` if that's legal too

Done.