Hello, To be honest, I couldn't quite understand the proposal(s). For the sake of timezone we always want to give/receive as much feedback as possible in a review to reduce the number of iterations. Reviews are also a means to learn -- so sometimes small comments (or answers to reviewers questions) are helpful too.
It seems that your concern is that sometimes reviews start with a lot of comments on the small matters, and that cause noise to the more essential issues. Is that the problem? If that's the case, I suggest instead of creating a rule or more complex review cycles, as contributors, applying certain strategies that will "lead" the reviewers towards reviewing the essential. As usual, they are better served with a grain of salt. (1) Explicitly ask questions to the reviewers. If there are points of uncertainty, explicitly ask questions right when uploading PR. "I'm using base::foobar() is that a good approach or should I do something else?", "I'm adding these to XYZ, is that the right place?". This will ensure that issues will be considered. Comments like "Could you ignore the minor details since this is a draft and review whether the solution makes sense first?" in the PR are also OK, and gives the reviewer more context to work on. (2) Make "essentials" more explicit in your code. If it's hard for the reviewer to understand the overall design, he/she may delve into the . If it's a new feature, probably there's an adequated place in the code where it makes sense to explain it in more detail. Good class comments, explaining what's the role of the classes you are adding, how it relates to the other classes. Here you are explaining the essentials, and reviewers will pay attention. I know some people have a bias against comments in code, I kind of agree in many cases, but I think that class comments are a positive addition and their benefits outweight the potential redundancy and risk of "getting outdated". (3) Use smaller PRs to help your main PR. Many contributions I make, consist of "a main change" and various "supporting changes" around. Sometimes these supporting changes are valid on their own, regardless of your original main change. Use that to your advantage, make a smaller patch that does that change and upload it into a separate PR. You can reference the reviewer in the original PR saying: "hey those patches here are in another PR already". For those who tracked, the development and landing of SysApps "Raw Sockets" is one example of this approach: it started as a patch series, that was broken down into smaller patches, that ended up going to their own PR. In the end, the PR contained only the actual implementation of TCPSocket object. Not all "supporting changes" are fit for this approach. Some of them depends on agreeing on a design, but sometimes is easy to get things going with the small changes. The whole point is that if the PR is smaller, it's easier to see the essential and discuss about it. Cheers, Caio _______________________________________________ Crosswalk-dev mailing list [email protected] https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev
