Lawrence Mitchell <[email protected]> writes: > 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.
I would say that integrators should make sure that they have the post-reviewed version of the branch when they ultimately merge. For example, I do something like $ git fetch bitbucket:author/dolfin.git a-branch-name:author/better-branch-name $ git log -p master..author/better-branch-name ... write comments on PR $ git branch -D author/better-branch-name Author updates PR in response to review, I repeat the above, then $ git checkout next $ git merge author/better-branch-name $ git push origin next author/better-branch-name If I wanted to perform some fix-up (typo, better commit message), I would notify the author of the PR. They can update the PR by resetting it to 'author/better-branch-name' from upstream (indicating approval of my minor changes), in which case the PR becomes "merged", otherwise we have to mark the PR as "declined" and give a note indicating that it was actually merged. (This is my biggest complaint about bitbucket PRs. With PETSc, most branches are in the same repository, so the integrator can do fix-up and updating the PR themselves.) The alternative to integrator fix-up is to always make it a separate commit on top of the PR. I like the cleaner history of not having "fix minor flaws in the last five commits" and you can't fix-up commit messages, which is my most common form of integrator fix-up. Anyway, after a commit is merged, the original author can do $ git checkout a-branch-name $ git fetch upstream $ git rebase upstream/author/better-branch-name This will be a no-op if the integrator didn't perform any fix-up, otherwise we'll see what is different. Future bug fixes on this topic can go here. _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
