Hi Halton, I don't suggest submitting patches before "intent to implement". The suggestion is to upload WIP patches (but mature enough to see which objects are doing what) after "intent to implement" in order to see and fix the design issues ASAP. This is much better comparing to the situation when the patch is complete in terms of functionality but needs rewriting as the design is not appropriate. I believe we could save a lot of time and efforts figuring out the design issues at early stages.
Off course this is needed only if the patch contains significant changes (new functionality). BR, Mikhail ________________________________________ From: Huo, Halton Sent: Monday, November 18, 2013 5:54 PM To: Pozdnyakov, Mikhail; [email protected] Subject: RE: Accelerating the review process Hi Mikhail, I do not quite get what you're suggesting? Submit prototype code before design (intent to implement)? As for the #3 part, I think we already follow the review-rework cycle until get LGTM. Thanks, Halton. > -----Original Message----- > From: Crosswalk-dev > [mailto:[email protected]] On Behalf Of > Pozdnyakov, Mikhail > Sent: Friday, November 15, 2013 10:39 PM > To: [email protected] > Subject: [Crosswalk-dev] Accelerating the review process > > Hi there, > > Ming Bai and me had recently a discussion about accelerating the review > process (which is sometimes quite painful and time-consuming at the > moment). > > We have ended up with some notes/proposals that as I guess can be > shared among the other developers > > 1) Basic rule is that all the essential issues should be pointed out ASAP > during the initial review and then the corrected code won't take too much > time to check before merging (it is supposed to contain only minor issues). > > 2) However, sometimes it is pretty hard to follow up the item #1, especially > if patch contains implementation of a new feature. It might take several > iterations to figure out an appropriate design for the new functionality > (You would agree that making sure that the intended design is good (and > often guiding on making an appropriate design) is the most important and > the most difficult part of code review) > > 3) In order to resolve the item#2 we're proposing the following > procedure: > > we split our review-modify process into two steps, first, we push the > code, which may be WIP but enough for getting a whole picture of the > design, > then we could start the review -- only focusing on the design until all the > following patches are uploaded and the design is good. Then we could > start the > next step, to address the minor issue and fix it. We could write some > comments > in the PR to indicate which step we are currently in, for example like > "okay > ,let's start to review the design." Or "The design is okay for me, so let's > start to > figure out the minor issues" and in the end, "LGTM if those minor issues > are > fixed.", and we'll give comments like "Please review the design." Or "I've > fixed > the minor issues, please have a look if it's good to merge?" I believe the > more > we developers and reviewers know about each other's progress, the > more > efficient we would be. > > Please tell what you think about the proposals above. > > BR, > Mikhail > --------------------------------------------------------------------- > Intel Finland Oy > Registered Address: PL 281, 00181 Helsinki Business Identity Code: > 0357606 - 4 Domiciled in Helsinki > > This e-mail and any attachments may contain confidential material for the > sole use of the intended recipient(s). Any review or distribution by others > is strictly prohibited. If you are not the intended recipient, please contact > the sender and delete all copies. > > _______________________________________________ > Crosswalk-dev mailing list > [email protected] > https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Crosswalk-dev mailing list [email protected] https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev
