Yes Johan
On 05/24/2013 02:05 PM, Nico Schlömer wrote: > Whatever strategy is recommended, could that be put on the FEniCS website? > > --Nico > > > > On Fri, May 24, 2013 at 2:00 PM, Johan Hake <[email protected] > <mailto:[email protected]>> wrote: > > Lawrence! > > Thanks for this write up. We will try publish a version of this > description on fenicsproject.org <http://fenicsproject.org> for best > git practice. > > On 05/24/2013 01:47 PM, Martin Sandve Alnæs wrote: > > I think it's best to avoid squashing and other commit id modifying > tasks > > after the branch has been made public. That's my understanding of > where > > this went wrong, but I must admit I haven't looked in much detail. > If no > > commit ids are rewritten, fixes can be commited to both > > of dolfin.git/feature-branch and > jrandom-developer.git/feature-__branch > > and merged between them, then merged into next again and finally into > > master, I see no issues then. > > That was my impression too. > > > Martin > > > > > > On 24 May 2013 13:35, Lawrence Mitchell > <[email protected] <mailto:[email protected]> > > <mailto:[email protected] > <mailto:[email protected]>>> wrote: > > > > Dear all, > > > > I recently contributed some code to dolfin which raised some > > questions about how best to carry out pull request/code review and > > subsequent merging. As an outside developer (with no access > to the > > main dolfin repository) here is how I envisaged the process > working: > > > > 1. Fork dolfin.git > > > > 2. Hack away implementing new feature. > > Should add that this should be done in its own feature branch. > > > 3. When ready submit as pull request. > > > > 4. Respond to review comments by making changes in my branch. > > > > Where appropriate, these would either be as new commits, or > > fixups squashed into existing commits (perhaps fixing a typo). > Like > > a good git citizen, I rerolled my patches in response to review > > comments. > > Why is it bad git habit to push small typo commits? Especially when it > potentially will screw the history compared to an already published and > pushed branch on the main dolfin repository? > > > 5. Force push latest version out for people to look at. > > > > 6. When everyone is happy, push final version ready for merging. > > > > This seems to conflict slightly with the integration testing that > > maintainers are doing, as a result of which, bitbucket got in a > > terrible tizz (see discussion here > > > > https://bitbucket.org/fenics-__project/dolfin/pull-request/__24/expose-petsc-objects-to-__pydolfin-using/activity > > > > <https://bitbucket.org/fenics-project/dolfin/pull-request/24/expose-petsc-objects-to-pydolfin-using/activity>). > > > > AIUI, the dolfin maintainers do something like the following: > > I change this to: > > 1. Fetch feature branch into local repo > > 2. Test the feature branch locally > > 3. Push to main repo > > 4. If there are issues, either ask jrandom to update his > branch or just fix it in the feature branch > > 5. Pull any new fixes from jrandoms repor > > 6. Graduate to next > > 7. Do 4-6 until satisfaction. > > 8. Graduate to master > > Johan > > > 2. Merge feature branch to next and test > > > > 3. Fix any minor issues with feature branch > > > > 4. Eventually merge to master > > > > > > This works as long as, before the merge to master, > > dolfin.git/feature-branch and > jrandom-developer.git/feature-__branch > > agree on the commit ids. Remember, bitbucket still thinks the > merge > > is coming from the latter repository. > > > > So the problem seemed to arise in the interaction between my > feature > > branch, and the feature branch pulled in to the main > repository. I > > was expecting that it would be reset to whatever I had pushed > out in > > response to review, whereas this didn't happen remotely. > > > > Is there any strong feeling and/or guidance on what I should > have done? > > > > Lawrence > > > > -- > > The University of Edinburgh is a charitable body, registered in > > Scotland, with registration number SC005336. > > > > _________________________________________________ > > fenics mailing list > > [email protected] <mailto:[email protected]> > <mailto:[email protected] <mailto:[email protected]>> > > http://fenicsproject.org/__mailman/listinfo/fenics > > <http://fenicsproject.org/mailman/listinfo/fenics> > > > > > > > > > > _______________________________________________ > > fenics mailing list > > [email protected] <mailto:[email protected]> > > http://fenicsproject.org/mailman/listinfo/fenics > > > > _______________________________________________ > fenics mailing list > [email protected] <mailto:[email protected]> > http://fenicsproject.org/mailman/listinfo/fenics > > _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
