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.
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.
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).
AIUI, the dolfin maintainers do something like the following:
1. Pull feature branch into main repo
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]
http://fenicsproject.org/mailman/listinfo/fenics
