Subject: Re: [boost] [pimpl] No documentation for pointer semant
From: Dave Gomboc (dave_gomboc_at_[hidden])
Date: 2014-06-07 18:19:33


[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