On 24 May 2013 13:28, Johan Hake <[email protected]> wrote: > 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? >>
I would suggest using the Bitbucket wiki util the procedure crystallises. We can also lift some of the instructions from the PETSc wiki. Garth >> --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 _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
