Re: [boost] [pimpl] No documentation for pointer semant
[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.
[Dave Gomboc]:
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.
[Andrey Semashev]:
I think a tag would be a better alternative to a SHA. It's more readable and GitHub creates a downloadable archive for it so git is not required to review the library.
Sure, using a tag would be great. (I wasn't aware that GitHub had that additional functionality.) Mainly, I have concern that Boost might adopt a procedure whereby committing to master is not permitted during a review. I want to discourage the mindset that changes can't be committed to their logical place (or even at all) just because a review is currently underway, and so tried to show that it is not difficult to avoid such an encumberance. Dave
participants (1)
-
Dave Gomboc