I would prefer all patches to have reviewboard, but for a while I remember there was the discussion about Drill is in rapid development and much easier to have progress without waiting for review.
Most of our reviewboard iiuc don't have ship its before it's merged even, I would think we need guidelines to include the whole process especially when the committer list grows that spans multiple companies. Tim Sent from my iPhone > On Oct 23, 2014, at 9:11 PM, Jacques Nadeau <[email protected]> wrote: > > I don't think we have a firm set of guidelines. We should probably come up > with something. My personal thought is something along the lines of more > than 2 files or 4 real lines of code change per file requires a > reviewboard. Additionally, if the patch isn't a straight commit, you > should provide the feedback on the jira. Since we have the patch review > tool, reviewboard shouldn't be a big deal, right? > > Do other people have thoughts on this? > > thx, > j > > >> On Thu, Oct 23, 2014 at 8:51 PM, Aman Sinha <[email protected]> wrote: >> >> The 20KB size of the patch seems misleading since a good part of it was >> unit test files. I reviewed the 1st patch and provided Hanifi some verbal >> comments. I could have put the comments in the bug but wasn't aware that >> we are required to do that unless the patch is big enough for a review >> board. I probably should have at least done a +1 to indicate someone >> looked at it. The revised patch looked ok to me and I committed it. We >> can create new JIRA if needed to track modifications... >> >> Aman >> >> On Thu, Oct 23, 2014 at 7:56 PM, Jacques Nadeau <[email protected]> >> wrote: >> >>> Hey guys, >>> >>> I'm concerned about the patch for 1547 that just went in. I think that >> the >>> functionality may be incorrect or duplicative. However, I didn't see >> this >>> patch up on reviewboard before it was merged. >>> >>> I'm all for no reviewboard for a few line patch but with a 20k patch, we >>> should have a review and I didn't see one. Hanifi, can you please post >>> this up for review. If everybody is okay, then fine. Otherwise, we can >>> track any fixes as an additional JIRA. >>> >>> thx, >>> Jacques >>
