Hi Cristina, SFC has been doing this from the beginning and I think it is a great idea which should be enforced. There are of course exceptions which should be handled with a workaround, especially at the beginning, but as you say, they should be limited on time. When a person merges his own patches, there are some dangerous potential consequences which jeopardize the values of community driven development, for example: 1 - Single Point of Failure. That person is the only one that knows about some piece of the code. If that person stops working in the project, can other people continue the development?2 - Coding hero/god mentality which is very bad for the health of the community and will frighten potential new committers or consumers of that project3 - Quality of code becomes worse. Several eyes on the code is better than just two. Jenkins tests will give +1 to spaghetti code4 - Knowledge sharing is stopped. By code reviews, developers share knowledge and ideas or can enforce good documentation5 - If I am PTL and I am able to merge my own patches. Why would I want to add new committers?6 - Company driven projects instead of community driven projects Unfortunately, I was not able to join Tuesday's or Thursday's meeting because of a business trip. Did you reach any agreement? By reading your mail it seems there were people against it which suprises me because I see this as common sense in open source communities but perhaps I am missing the points of the people against it. Regards,Manuel On Wed, 2019-02-06 at 15:17 +0000, Cristina Pauna wrote: > Hi all, > > The first proposal from the > Code and Release Quality (Improve project's info accuracy) has been > approved by the TSC on 5th Feb > (see TSC-14 > for details). > > The second proposal (Improve gerrit best practices), although > generally well received, has raised some concerns and I would like to > ask the community for more feedback on it. > The proposal is to discourage self-merging patches by adding this > into our wiki documentation and, in a reasonable timeline to enforce > it through gerrit (current recommendation is Hunter 8.0 milestone) > > The background for this proposal is good practices done in other open > source communities, which value peer review. Some examples are: > · > OpenStack requires 2 committers to give a +2 for any patch: > https://docs.openstack.org/infra/manual/developers.html#code-review > · > Akraino requires contributor diversity (the project demonstrates that > it has a stable core team of contributors/committers > which are affiliated to a set of at least three different > companies): > https://wiki.akraino.org/display/AK/Akraino+Technical+Community+Docum > ent#AkrainoTechnicalCommunityDocument-3.3.7.3CoreReview > · > Blog with gerrit best practices that promotes to not self-approve > changes and to have at least 2 other people review the change: > https://blog.simplypatrick.com/html/gerrit-best-practices.html > · > Some OPNFV projects already follow this good practice although is not > enforced by gerrit > > > Peer reviews are important because they make other members to better > understand the changes that are happening in a project, give > opportunity to have architectural discussions, make committers > more involved and responsible, and improve the overall quality of a > project. > > > Understandably, there have been situations when a project had a > single active committer, and those can be excepted from the rule for > a limited period of time. On the long run this type of projects > have been proven to be unsustainable anyway (most of them died on > their own when the person sustaining the project pursues other > interests). Exceptions can always be approved if there’s a good > reason for them. > > These are some concerns I got over this proposal: > - > Code is automatically verified by Jenkins so a Verified +1 from > Jenkins assures the new code is not breaking anything > > o > Peer review is just as important as automatic verification of the > patches, as it provides all the benefits listed above. One should not > exclude the other > - > Trivial patches don’t need peer review > > o > What is trivial and not can be better decided by a second set of eyes > (one line change can have major impact). Also, it should not matter > if a change is trivial or not to have proper peer > review > - > Low number of active developers in OPNFV > > o > This is indeed a concern and a reason for which this practice has > become so common. But unless a project has just 1 active committer, > self-approved patches should not happen > > I would like to know if your projects already has this practice > informally, if you would like to have it as a hard requirement, and > if you have any concerns. You can either use this thread or join > us on the Technical Discuss meeting this Thursday (https://wiki.opnf > v.org/display/PROJ/Weekly+Technical+Discussion) > > Thanks, > Cristina > > > > > From: opnfv-tech-discuss@lists.opnfv.org [mailto:opnfv-tech-discuss@l > ists.opnfv.org] > On Behalf Of Cristina Pauna > > Sent: Friday, January 25, 2019 11:08 AM > > To: opnfv-tech-discuss@lists.opnfv.org > > Subject: [opnfv-tech-discuss] Code and Release improvements > > > > Hi, > > During the end of last year, the TSC formulated the OPNFV’s vision > for 2019 to move to a more devops approach. An area identified to be > improved was Code and Release, and a process was devised to implement > these improvements: > > > Gather the proposals from the community in a template format that > would provide clarity and uniformity (see format and examples in the > etherpad)Debate the proposals in the Technical meeting and get them > into a final form. At this time there has to be an owner committed to > implement/drive the proposalPresent the proposals that have a final > form to the TSC meeting and get approval from the TSC (things can go > back and forth and be re-considered if the TSC doesn’t approve)Create > a Jira for each proposal in the TSC project to track progress of the > implementationMonitor that the proposal is implemented, add comments > to the Jira with the progress > > Etherpad > https://etherpad.opnfv.org/p/Code_and_Release_Quality was created to > gather and debate the proposals. For those of you who want to > contribute to the improvement of the code and release, go ahead and > add your proposals there, as well as add your notest on > the the existing ones (make sure you add your name to your notes). > Looking forward to see your input. > > Thanks, > Cristina > > > This message, including attachments, is CONFIDENTIAL. It may also be > privileged or otherwise protected by law. If you received this email > by mistake please let us know by reply and then delete it from your > system; you should not copy it or disclose its contents > to anyone. All messages sent to and from Enea may be monitored to > ensure compliance with internal policies and to protect our business. > Emails are not secure and cannot be guaranteed to be error free as > they can be intercepted, a mended, lost or destroyed, > or contain viruses. The sender therefore does not accept liability > for any errors or omissions in the contents of this message, which > arise as a result of email transmission. Anyone who communicates with > us by email accepts these risks. > > > > > > This message, including attachments, is CONFIDENTIAL. It may also be > privileged or otherwise protected by law. If you received this email > by mistake please let us know by reply and then delete it from your > system; you should not copy it or disclose its contents > to anyone. All messages sent to and from Enea may be monitored to > ensure compliance with internal policies and to protect our business. > Emails are not secure and cannot be guaranteed to be error free as > they can be intercepted, a mended, lost or destroyed, > or contain viruses. The sender therefore does not accept liability > for any errors or omissions in the contents of this message, which > arise as a result of email transmission. Anyone who communicates with > us by email accepts these risks. > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > > View/Reply Online (#22768): https://lists.opnfv.org/g/opnfv-tech-disc > uss/message/22768 > Mute This Topic: https://lists.opnfv.org/mt/29535315/675458 > Group Owner: opnfv-tech-discuss+ow...@lists.opnfv.org > Unsubscribe: https://lists.opnfv.org/g/opnfv-tech-discuss/unsubb [mb > u...@suse.com] > -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group.
View/Reply Online (#22778): https://lists.opnfv.org/g/opnfv-tech-discuss/message/22778 Mute This Topic: https://lists.opnfv.org/mt/29535315/21656 Group Owner: opnfv-tech-discuss+ow...@lists.opnfv.org Unsubscribe: https://lists.opnfv.org/g/opnfv-tech-discuss/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
