Hi Jason, I think this is a good idea that should speed up the review process slightly.
Maintainers will still have to re-ack the change, which will give them the opportunity to decide if a revision is minor enough not to require a new review. The only drawback I can think of is that there could be corner cases where reviewers and maintainers have different opinions about what constitutes a minor change. IIRC, we treat reviews as sticky for simple rebases where the code doesn't change. We could make them sticky for commit message changes as well, but that would only partially solve problem Jason is highlighting. Cheers, Andreas On 11/09/2017 16:40, Jason Lowe-Power wrote:
Hi all, Currently, if there is a change to a patch on gerrit, all of the previous tags are cleared. For instance, if you upload a patch and receive a +2 review, then you make a minor update (or any update) the +2 is lost. I'm proposing changing this rule to only clear Maintainer +1 and keep all +2's. Thus, for small changes you will no longer need to re-review the patch. The only downside to this is if someone makes a major change after you have given a +2, you will have to notice and remove your +2 if you disagree. Otherwise, I am hoping this will change will make things a little easier on reviewers. Anyone have any objections (or other opinions)? If I don't hear anything for a week we'll go ahead and make the change. Cheers, Jason _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
