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 > > >
