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
