[Paul A Bristow]:
I unhappy with a review rigid policy that nothing can be changed during a review. It isn't always helpful.
We never accept anything without requiring some changes?
Can we instead now use git's easy branches to specify a fixed 'master' branch, but allow the author to update a 'develop' branch as the review develops?
[Edward Diener]:
I believe that changes can be made to a reviewed library during a review as long as the 'master' branch does not change. After all we want all reviewers to be looking at the same 'master' branch as the code to be reviewed. But if the developer wants to make changes to other branches during the review I think this should be allowed.
May I suggest that either the library author or the review manager ought to, near the commencement of the review, identify a specific version to be reviewed? This is quite distinct from identifying a branch name for a conventional branch such as master that is expected to change frequently even over a two-week period. A specific checkout is typically identified by a SHA: a branch name such as 'master' is primarily just an alias to the SHA to which it currently resolves. Then again, Boost is relatively new to using git, so perhaps a new branch name can be introduced to refer specifically to the review version. The review manager could, at the commencement of the review, specify the checkout instructions using a specific SHA. For example, for Boost.Pimpl, the manager would provide Source: https://github.com/yet-another-user/boost.pimpl, SHA 59a5c0d or, in command-line-ese, git clone https://github.com/yet-another-user/boost.pimpl git reset --hard 59a5c0d As I mentioned, not everybody may be comfortable working directly with SHAs immediately, so the library author and/or the review manager may wish to introduce a specific label for the purpose of easing identifying the review version. If one of them (or somebody else with write access to the repository) continues from the above two commands with the further git checkout -b boost_review git push origin boost_review then it still becomes possible for the contents of the master branch to change (which, generally speaking, is highly desirable) without disrupting the review process. In this case, the instructions provided by the manager could be: Source: https://github.com/yet-another-user/boost.pimpl, branch boost_review or, in command-line-ese, git clone https://github.com/yet-another-user/boost.pimpl git checkout boost_review which should be more palatable to the large group of people migrating from Subversion. Naturally, in this case it is imperative that boost_review not itself be pushed to further during the review. However, there is no reason the library author from continuing with these commands: git checkout -b boost_review_fixes git push -u origin boost_review_fixes at which point, fixes to items that come up in the review can be pushed onto that branch, and those people who specifically want to take a look at the fixes can easily do so. The above said, the long-term simplicity of specifying a SHA directly instead of a branch at all is not only that one doesn't have to go and make a bunch of new branch names in order to continue development, but even more importantly, one can't push to a SHA to change its contents --because modifying a commit after the fact modifies its SHA also -- so everyone is sure to get the same version of the content being reviewed. Dave