Re: [Cython] Git workflow, branches, pull requests

2011-05-13 Thread Dag Sverre Seljebotn

On 05/13/2011 12:36 AM, Ondrej Certik wrote:

Hi,

On Thu, May 5, 2011 at 12:52 PM, Dag Sverre Seljebotn
d.s.seljeb...@astro.uio.no  wrote:

There was just a messup in git history: Mark's OpenMP pull request got
merged twice; all commits show up two times.

It doesn't matter, since the two openmp branches with the same changes
merged OK, but we shouldn't make this a habit. For instance, the openMP
commits also show up as part of vitja's pull request, which is confusing.

In Mercurial speak: The openmp branch was used like you would use a
Mercurial patch queue in one case, and as a branch in another case. In git
they are the same technically and you rely on conventions to make sure you
don't treat a queue as a branch.

OPTION A) Either i) only branch from master, or ii) make sure you agree with
whoever you're branching from that this is a branch, not a patch queue,
so that it isn't rebased under your feet.

We could also, say, prepend all patch queues with an underscore (its
private).

OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but
nobody are pulling from dagss/cython these days anyway.


What about:

OPTION C) The one who pushes things into the master knows master
enough to see whether or not it makes sense to merge this, or if it
was already in, he/she will simply comment into the pull request and
close it manually


This doesn't make sense to me. Are you sure you read the scenario correctly?

Dag Sverre
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-13 Thread Dag Sverre Seljebotn

On 05/13/2011 09:05 AM, Ondrej Certik wrote:

On Thu, May 12, 2011 at 11:34 PM, Dag Sverre Seljebotn
d.s.seljeb...@astro.uio.no  wrote:

On 05/13/2011 12:36 AM, Ondrej Certik wrote:


Hi,

On Thu, May 5, 2011 at 12:52 PM, Dag Sverre Seljebotn
d.s.seljeb...@astro.uio.nowrote:


There was just a messup in git history: Mark's OpenMP pull request got
merged twice; all commits show up two times.

It doesn't matter, since the two openmp branches with the same changes
merged OK, but we shouldn't make this a habit. For instance, the openMP
commits also show up as part of vitja's pull request, which is confusing.

In Mercurial speak: The openmp branch was used like you would use a
Mercurial patch queue in one case, and as a branch in another case. In
git
they are the same technically and you rely on conventions to make sure
you
don't treat a queue as a branch.

OPTION A) Either i) only branch from master, or ii) make sure you agree
with
whoever you're branching from that this is a branch, not a patch
queue,
so that it isn't rebased under your feet.

We could also, say, prepend all patch queues with an underscore (its
private).

OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but
nobody are pulling from dagss/cython these days anyway.


What about:

OPTION C) The one who pushes things into the master knows master
enough to see whether or not it makes sense to merge this, or if it
was already in, he/she will simply comment into the pull request and
close it manually


This doesn't make sense to me. Are you sure you read the scenario correctly?


You wrote:


There was just a messup in git history: Mark's OpenMP pull request got
merged twice; all commits show up two times.


So somebody pushed in Marks' patches twice. My OPTION C) is that the
one, who pushes patches in is responsible to make sure that they only
get pushed in once.

That's what we do in sympy, we don't have any formal option A or B,
but people with push access must prove that they are capable of using
git, and not breaking (or messing up) things. Of course, everybody can
make a mistake though.

It seems to be working just great, so I just wanted to share our
experience. Let me know what doesn't make sense.


Ah ok. So in this case, the reviewer would have to request that the 
second pull request was fixed/rebased. I guess that is still the safety 
mechanism, but it's nice to also discuss how to not get into those 
situations. I'm not saying that there will be serious repercussions if 
one doesn't follow the rules, I was more talking guidelines for not 
getting into trouble without having to learn all of git.


Note that a big part of this thread was to actually make sure everybody 
(in particular the core devs) knew about how Git rebasing works. The 
mistake was made in the first place because the reviewer assumed that 
git will figure this out. Keep in mind that we just switched, and I 
think some core devs are still using hg-git, for instance.


Dag Sverre
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-06 Thread Dag Sverre Seljebotn

On 05/05/2011 11:07 PM, Robert Bradshaw wrote:

I don't like the default to be don't pull from me--I'd rather there
be some convention to indicate a branch is being used as a queue.
Maybe even foo-queue, or a leading underscore if people like that.


I've seen leading underscore being used by other people on github, so 
let's settle on that for now.


Of course, if you do pull from a non-master branch, you should be 
communicating a lot about that fact anyway; it's a bad idea for a lot of 
other reasons as well.


I've updated http://wiki.cython.org/HackerGuide.

Here's an example of prior art in git workflows, developed I think 
primarily for IPython:


https://github.com/matthew-brett/gitwash

It's essentially some Sphinx documentation with replacable names (To 
contribute to PROJECTNAME, you should get an account on github...) that 
one can merge into ones own documentation. If anybody is interested in 
looking at that I'm all for it.


Dag Sverre
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-06 Thread Vitja Makarov
2011/5/6 Dag Sverre Seljebotn d.s.seljeb...@astro.uio.no:
 On 05/06/2011 08:20 AM, Vitja Makarov wrote:

 2011/5/6 Robert Bradshawrober...@math.washington.edu:

 I don't like the default to be don't pull from me--I'd rather there
 be some convention to indicate a branch is being used as a queue.
 Maybe even foo-queue, or a leading underscore if people like that.

 On Thu, May 5, 2011 at 2:03 PM, Dag Sverre Seljebotn
 d.s.seljeb...@astro.uio.no  wrote:

 Yes, that is the only time it happens.

 Do we agree on a) ask before you pull anything that is not in cython/*
 (ie
 in private repos), b) document it in hackerguide?

 DS


 --
 Sent from my Android phone with K-9 Mail. Please excuse my brevity.

 Robert Bradshawrober...@math.washington.edu  wrote:

 On Thu, May 5, 2011 at 1:22 PM, Stefan Behnelstefan...@behnel.de
  wrote:

 Dag Sverre Seljebotn, 05.05.2011 21:52:    There was just a messup
 in

 git history: Mark's OpenMP pull request got  merged twice; all
 commits
 show up two times.    What (I think) happened, was that Vitja pulled
 in
 Mark's changes into his  unreachable code removal branch, and they
 ended up
 in his pull request. I  guess I was assuming that git wouldn't care
 too
 much about branch  duplication, so I just accepted the pull request
 via the
 web interface.  Apparently, it did care.    I tend to rebase my
 local
 change sets before pushing them, and I think it  makes sense to
 continue
 doing that. +1, I think for as-yet-unpublished changes, it makes the
 most
 sense to rebase, but for a longer-term branch, merging isn't as
 disruptive
 to the history (in fact is probably more reflective of what's going on)
 and
 is much better than duplication. To clarify, is this only a problem
 when we
 have A cloned from master B cloned from A (or from master and then
 pulls in
 A) A rebases A+B merged into master ? If this is the case, then we
 could
 simply make the rule that you should ask before hacking a clone atop
 anything but master. (Multiple people can share a repeatedly-rebased
 branch,
 right.) We could also us the underscore (or another) convention to mean
 this branch is being used as a queue, puller beware. Surely other
 projects
 have dealt with this. - Robert


 About my branch:

 I've rebased it from upstream/master at home and made forced push
 At work I pulled it back and rebased from origin, then I tried to
 rebase if again from upstream/master

 Do I understand correctly that you:

  a) You make local changes at home
  b) Rebase them on cython/master
  c) Force-push to vitja/somebranch
  d) Go to work, where you have other local changes
  e) Rebase your work changes at work on top of vitja/somebranch



Right.

 If this is correct; then this can't work. The reason is that after the
 force-push in c), there are no shared commits (apart from what's shared from
 cython/master) between your work computer and vitja/somebranch.

 So the rule is: If you rebase a branch, then if you have other copies of
 that branch (like on a work computer), destroy them (e.g., git branch -D)!
  And then fetch new copies of the branches.

 (And as you say, if you do have different changes in many places then you
 can recover from an unfortunate rebase by cherry-picking. And you can always
 undo a rebase by looking at git reflog and manually check out the old
 HEAD.)


Thank you for explanation.

So btw, when I do rebase and my changes were already pushed I have to
use forced push.
Is forced push ok?

-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-06 Thread Yury V. Zaytsev
On Thu, 2011-05-05 at 13:49 -0700, Robert Bradshaw wrote:

 Surely other projects have dealt with this.

We have for instance adopted a very strict set of policies on that:

1) Never branch of anything except for master or x.y.z-stable branches
2) Never rebase master or x.y.z-stable branches, unless everybody agrees
3) Never do fast-forward merges, always rebase your stuff before merge
4) Never commit directly to master, use feature branches

That is, all branches are either in the personal namespace of the
developers for experimental stuff, or in the main namespace prefixed
with the ticket number.

I admit that this is quite restrictive, but in the end, it doesn't take
so much effort to follow and the history is very nice and easy to
understand, I even literally follow the development by reading the
history, when I am unable to track IRC etc.

Of course, you don't have to copy this blindly, but few permissive
conventions, which do not impose additional load on the developers like

1) Always branch from master
2) Never rebase master

would definitively not hurt.

I was recently trying to bisect Cython, as I was under impression that
recent commits broke something for me (it turned out not to be the case,
but that's not the point) and the history is just to horribly confusing
that I couldn't make any sense out of it.

All these branches from branches from branches which gets merged in
random order make it impossible for an outside person to make any
sensible use of this information...

Hope that helps,

-- 
Sincerely yours,
Yury V. Zaytsev


___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-05 Thread mark florisson
On 5 May 2011 21:52, Dag Sverre Seljebotn d.s.seljeb...@astro.uio.no wrote:
 There was just a messup in git history: Mark's OpenMP pull request got
 merged twice; all commits show up two times.

 It doesn't matter, since the two openmp branches with the same changes
 merged OK, but we shouldn't make this a habit. For instance, the openMP
 commits also show up as part of vitja's pull request, which is confusing.

 In Mercurial speak: The openmp branch was used like you would use a
 Mercurial patch queue in one case, and as a branch in another case. In git
 they are the same technically and you rely on conventions to make sure you
 don't treat a queue as a branch.

 OPTION A) Either i) only branch from master, or ii) make sure you agree with
 whoever you're branching from that this is a branch, not a patch queue,
 so that it isn't rebased under your feet.

 We could also, say, prepend all patch queues with an underscore (its
 private).

 OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but
 nobody are pulling from dagss/cython these days anyway.

 Opinions?

 FYI,

 The workflow me and Mark is currently using is:

  a) Fork off a feature branch from master (with master I'll always refer to
 cython/master)

  b) When one gets in sync with master, do NOT merge master, but rather
 rebase on top of it:

     git pull --rebase origin master

  c) Continue rebasing, and eventually .

 The advantage of this approach is that ugly merges disappear from history,
 since commits are rewritten. And the history graph looks very nice and is
 easy to follow.

 BUT, if the result is duplication, we should avoid this practice, and rather
 always merge.


 Dag Sverre
 ___
 cython-devel mailing list
 cython-devel@python.org
 http://mail.python.org/mailman/listinfo/cython-devel


I think the rebasing is pretty elegant, so I'm +1 on that, as long as
everyone agrees because those duplicated commits are nasty. I'm
surprised git didn't issue an error to prevent this.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-05 Thread mark florisson
On 5 May 2011 22:22, Stefan Behnel stefan...@behnel.de wrote:
 Dag Sverre Seljebotn, 05.05.2011 21:52:

 There was just a messup in git history: Mark's OpenMP pull request got
 merged twice; all commits show up two times.

 What (I think) happened, was that Vitja pulled in Mark's changes into his
 unreachable code removal branch, and they ended up in his pull request. I
 guess I was assuming that git wouldn't care too much about branch
 duplication, so I just accepted the pull request via the web interface.
 Apparently, it did care.

 I tend to rebase my local change sets before pushing them, and I think it
 makes sense to continue doing that.

 Stefan
 ___
 cython-devel mailing list
 cython-devel@python.org
 http://mail.python.org/mailman/listinfo/cython-devel


Whatever approach we take, it should be documented extensively on the
hackerguide, at least the rebase approach, as it has apparently
somewhat catastrophic results if not everyone does it.

On the one hand rebasing gives you a nice history, but on the other
hand it's also a bit harder to use and if you start merging at some
point reverting to rebasing is not so easy. So perhaps we should just
ignore the 'merge' commits (using command line git, git log
--no-merges) and pull and push away.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Git workflow, branches, pull requests

2011-05-05 Thread Dag Sverre Seljebotn
Yes, that is the only time it happens.

Do we agree on a) ask before you pull anything that is not in cython/* (ie in 
private repos), b) document it in hackerguide?

DS


-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Robert Bradshaw rober...@math.washington.edu wrote:

On Thu, May 5, 2011 at 1:22 PM, Stefan Behnel stefan...@behnel.de wrote:  
Dag Sverre Seljebotn, 05.05.2011 21:52:   There was just a messup in git 
history: Mark's OpenMP pull request got  merged twice; all commits show up 
two times.   What (I think) happened, was that Vitja pulled in Mark's changes 
into his  unreachable code removal branch, and they ended up in his pull 
request. I  guess I was assuming that git wouldn't care too much about branch 
 duplication, so I just accepted the pull request via the web interface.  
Apparently, it did care.   I tend to rebase my local change sets before 
pushing them, and I think it  makes sense to continue doing that. +1, I think 
for as-yet-unpublished changes, it makes the most sense to rebase, but for a 
longer-term branch, merging isn't as disruptive to the history (in fact is 
probably more reflective of what's going on) and is much better than 
duplication. To clarify, is this only a problem when we have A cloned from m
 aster B
cloned from A (or from master and then pulls in A) A rebases A+B merged into 
master ? If this is the case, then we could simply make the rule that you 
should ask before hacking a clone atop anything but master. (Multiple people 
can share a repeatedly-rebased branch, right.) We could also us the underscore 
(or another) convention to mean this branch is being used as a queue, puller 
beware. Surely other projects have dealt with this. - 
Robert_
cython-devel mailing list cython-devel@python.org 
http://mail.python.org/mailman/listinfo/cython-devel 

___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel