Thanks for the clarifications Jed! Johan
On 05/24/2013 03:20 PM, Jed Brown wrote: > 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 > _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
