Like Aman says, I was given a verbal review. RB is fine as well. I can still go ahead and create one if need be.
-Hanifi On Thu, Oct 23, 2014 at 9:32 PM, Timothy Chen <[email protected]> wrote: > 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 > >> >
