On Fri, 2017-11-17 at 09:59 -0500, Dave Barach wrote: > Dear Marco, > > Good ideas. > > We could also consider public patches, -2ed by the submitter, with commit > headlines of the form "RFC: add the XYZ feature." +1
> The only downside: each new patch set will kick off the verify jobs. I wonder > if one could remove "fd.io JJB" quickly enough to keep that from happening. Somehow, I cannot remove the "fd.io JJB" job at all on my patch submission... :( > > Thanks... D. Cheers, Marco > > -----Original Message----- > From: Marco Varlese [mailto:mvarl...@suse.de] > Sent: Friday, November 17, 2017 9:49 AM > To: Luke, Chris <chris_l...@comcast.com>; Dave Barach <d...@barachs.net>; vpp- > d...@lists.fd.io > Subject: Re: [vpp-dev] Please Call DigSafe... > > Hi Chris / Dave, > > Few thoughts inline... > > On Fri, 2017-11-17 at 13:51 +0000, Luke, Chris wrote: > > Hi Dave, > > > > After spending a few minutes to work out that you were talking about a > > proposed patch and not something any of us had merged (and, especially not > > that I merged!), I see that what we need is a balance between not > > discouraging > > people to experiment, or submit their ideas, but to also steer people > > towards > > relevant leads before they get in too deep. > > > > Problem is, if people make huge patches before ever talking to someone, our > > first contact is when they submit it. The teaching moment is when the > > reviewer > > notices it. That is obviously too late for the first patch, but should help > > with subsequent work. > > > > This is why open source generally prefers people to keep their patches small > > and thematic; most reviewers tire of seeing many large patches when they are > > developed in isolation and are directionally unsound - to the point that > > they > > start to see the color bar in the review list and if it's yellow-or-worse, > > and > > not from someone they specifically associate with quality work, typically > > those submissions end up ignored. > > Maybe, a point to consider to come up with a "process" is whether the patch is > addressing existing code (e.g. bug-fix / enhancement) or is a brand-new > feature. > > I would expect - as you stated - to see small (small to be quantified) patches > to fix a bug in existing code and considerably bigger patches when adding in a > new feature although - sometimes - finding a way to split code into multiple > subsequent patches might be doable. Usually, the impact of a new feature > should > be considerably smaller (to existing code path executions) hence a big-fat > patch > might be acceptable... > > We could use the DRAFT feature in "git review". That is very similar to the > RFC > (Request For Comments) method used in other projects. It might be a decent way > to get early feedback and work on the design with key people... > > > > I don't think we have contribution guidelines for VPP or fd.io in general > > (apart from the style and doc guides); at least a very quick scan of the > > wiki > > was not fruitful. We should have somewhere to send new people (can we nudge > > people who login to Gerrit for the first time?), and also people whose first > > submission is unacceptable (too big, too complex, directionally unsound). > > And > > we as reviewers should remain vigilant and, importantly, consistent. > > Maybe, we could have a process which starts with a DRAFT-type patch to give > time > to core reviewers to understand what's going on. > Beside the patch size topic, another item I would consider is the > length/quality > of COMMENTS in the patch; it's pretty common (and not just to FD.io/VPP > projects) to see very brief comments to patches which can make reviewers' life > miserable (and after 2 days that patch-comment would mean nothing even to the > author). IMHO that should be a straight -1... > I am quite a new contributor to VPP but a process I built in my mind over the > past few weeks is split into two sections and looks like: > > == NEW FEATURE DEVELOPMENT == > 1) Reach out to core people (Dave / Damjan / Ed / Florin / Dave W / Chris / > etc.) to understand feasibility of the proposed feature; > 2) Write your patch (including unit-tests where applicable) > 3) Fix coding style (make fixstyle) > 4) Check your COMMENTS (do they reflect the code submitted? the longer the > explanation the better) > 5) Submit DRAFT patch to gerrit > 6) Address any early comments > 7) Go back to [2] or SUCCESS :) > > == BUG-FIX == > 1) Write your patch > 2) Run unit-tests locally > 3) Write a clear explanation for your patch in the COMMENTS identifying (where > applicable) any Jira ticket pointing to the issue > 4) Submit patch to gerrit > 5) Address any early comments > 6) Go back to [1] or SUCCESS :) > > > > Chris. > > Cheers, > Marco > > > > > > > -----Original Message----- > > > From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io] On > > > Behalf Of Dave Barach > > > Sent: Friday, November 17, 2017 7:45 > > > To: vpp-dev@lists.fd.io > > > Subject: [vpp-dev] Please Call DigSafe... > > > > > > Folks, > > > > > > At our next project meeting, I'd like to spend a few minutes talking about > > > a > > > good-news / bad-news situation affecting the vpp project. > > > > > > As the community has expanded, committers have begun noticing > > > unacceptable and unfixable patches in mission-critical code. Yesterday's > > > soap-opera episode involved the ip4/6 speed-paths. > > > > > > I think we should allocate a bit of meeting time for folks to talk about > > > what > > > they're trying to develop, with an eye towards engaging with relevant area > > > experts from the start. > > > > > > In most places in the US, folks planning to dig holes on their property > > > are > > > required to call 811 (DigSafe): to avoid hitting buried gas lines and > > > blowing up > > > the neighborhood. It seems like we need to create something > > > similar for the vpp project. > > > > > > Thoughts? > > > > > > Thanks... Dave > > > > > > _______________________________________________ > > > vpp-dev mailing list > > > vpp-dev@lists.fd.io > > > https://lists.fd.io/mailman/listinfo/vpp-dev > > > > _______________________________________________ > > vpp-dev mailing list > > vpp-dev@lists.fd.io > > https://lists.fd.io/mailman/listinfo/vpp-dev > > _______________________________________________ vpp-dev mailing list vpp-dev@lists.fd.io https://lists.fd.io/mailman/listinfo/vpp-dev
