On Fri, May 24, 2013 at 01:56:09PM +0100, Garth N. Wells wrote: > 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.
Yes, I think that's a good idea. Then everyone can contribute with modifications until it's set in stone. Then we add it to the web page to make it official. -- Anders > 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 > >> > > >> > > >> > _________________________________________________ > >> > 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 _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
