[Cython] Git workflow, branches, pull requests

[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.seljebotn at 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.seljebotn at 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?
>
> 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