Hi Ghee, Thanks for the comments, although I would have appreciated the comments coming earlier, when I posted it for discussion. That said, it's never too late to ask questions.
Responses inline. On Mon, 2006-10-23 at 11:49 +0100, Ghee Teo wrote: > Hi Laca, > > Thank you Laca for putting together this nice and simple code review > process > http://www.opensolaris.org/os/project/jds/documents/code_review/ > > I need some clarifications on these statements when taken together: > > * bug category owners must review all patches in their category and > respond. If the patch looks good and you have no comments, just > reply with "approve" > * reviews are non-blocking: you can go ahead and commit if you are > confident enough that the fix it good, but be aware that you may > need to revert or amend your changes if your peers or the upstream > community have issues with it > * you must address all questions or concerns > > Q1. Who is the final 'authoritive' approver? (e.g. bug category owner > approve a patch, but non category owner opposes it or raised questions > and concerns so is the patch approve or not?) I'd really like a consensus, that's why all questions need to be answered/dealt with. If the category owners approves but someone else notices problems with the change, it still needs to be answered and/or fixed. If there is no agreement on the list (although code changes should not be political), the responsible manager decides (but I would really prefer to avoid that situation). > Q2. If the submitter is going ahead to submit the patch anyway, what is > the point of addressing all questions and concerns? > (Okay, I am over-stressing here a bit, but this is one possible > interpretation of the statements). If the patch turns out to be bad, the submitted has to undo his/her changes. > With regard to this statement: > > * your changes are considered approved if no issues are raised > within 3 working days or if all issues raised are addressed > > This implies the lead time for a patch approval is 3 days. This is too > long.. I think. This means, if you submit a code review request and noone reponds within 3 days, you can go ahead and commit and shame on the reviewers. > I think if the category owner has approved it then it is considered > approved, however, others can still raise concerns and questions > (regardless of time) that still need to be addressed but do not let the > process stopped. Correct. The process is not stopped anyway. You don't even need to wait for the category owner's approval. > Ultimately any concerns raised on this list will most > likely to be raised by the community. Right, that's why patches submitted to the community need not be reviewed on this list. Thanks, Laca > Laszlo (Laca) Peter wrote: > > The JDS Core review process posted earlier is now in action. > > See http://www.opensolaris.org/os/project/jds/documents/code_review/ > > for the description of the process. > > > > A new mailing list called jds-review was created and all Sun > > contributors were subscribed. Anyone is welcome to join at > > http://mail.opensolaris.org/mailman/listinfo/jds-review > > > > Everyone, please start submitting your svn diffs to jds-review > > prior to committing. > > > > Thanks, > > Laca > > > > > > >
