I tend to agree with Lusheng and Dan. If the size of the contribution is more than a certain limit, we can provide guidelines for enabling easier code review. Some suggestions are provided here -
1. Accompany the contribution with UML class diagrams to help structure the code review. 2. Contributor can organize a workshop on zoom to walk through the details before the contribution can be independently reviewed. This can be done selectively for large code contributions, so that we strike a balance. Regards Aayush On 4 August 2017 at 21:24:38 IST, TIMONEY, DAN <dt5...@att.com> wrote: All, I wanted to second Lusheng’s excellent comments below, and add perhaps a bit more. I think it’s important to bear in mind that some of the code that is contributed might not be entirely new. For example, much of the seed code that we submitted for openecomp was existing code that we had developed and used internally within AT&T and then open sourced. It is likely that we’ll come across similar examples as ONAP continues to evolve, where one company or another finds that they have existing code that can be integrated into ONAP that they would like to contribute. So, while we absolutely should encourage developers to add code in small increments as they’re working (upstream first) but we should also not ask them to expend extra effort to break up contributions they’d like to make to meet some imposed limitation on number of lines. Instead, perhaps there is some other process that should be followed for such commits (e.g. some comments within the commit message indicating this code is in use today by company X and is being contributed to ONAP?).. I’m sure other open source projects have had similar block commits, so it would be good to hear how other projects handle those types of contributions. Dan Dan Timoney Principal Technical Staff Member AT&T Email : dtimo...@att.com<mailto:dtimo...@att.com> Office : +1 (732) 420-3226 Mobile : +1 (201) 960-1211 200 S Laurel Ave, Rm E2-2A03 Middletown, NJ 08873 From: <onap-tsc-boun...@lists.onap.org> on behalf of "JI, LUSHENG" <l...@research.att.com> Date: Friday, August 4, 2017 at 1:43 AM To: "onap-tsc@lists.onap.org" <onap-tsc@lists.onap.org> Subject: Re: [onap-tsc] Enforcing an "Upstream first" approach to ONAP ***Security Advisory: This Message Originated Outside of AT&T *** Reference http://cso.att.com/EmailSecurity/IDSP.html for more information. Dear TSC and ONAP community, As I am reading through this thread, I can certainly appreciate David’s good intention behind the suggestion. Perhaps this is a minority opinion here, but as a PTL and committer I have to say I am also concerned about “enforcing” things such as hard limit on submission LOC. 1. Contributions come in the shapes that they are in. Some large some small. Putting a submission size limit will not change that, but it would impact the integrity of large contributions though. Say someone wants to contribute a new complex function but LOC too large, it is not like he would be able to break this one large contribution into several smaller contributions. The more likely outcome is that he will simply break the contribution into several smaller commits, each smaller than the LOC limit but only fragment of the original contribution file tree. This would make committer’s job much harder because he now must review ALL of these commits together, equal amount of LOC, and spread in multiple submissions. Individual commits like these cannot be reviewed individually effectively because it is difficult to assess its value to the whole project, without mentioning that individual submission may even break the CICD and testing of the project. 2. The “commit small commit often” practice of devops really relies on the rich support of features and branches of git. We essentially have ONLY ONE branch, the master. This is the key reason for why I am not sure we should follow the devops manual to a tee here. None of those sophisticated branching workflows is applicable here. ONAP gerrit is not best suited for being used as dev repo. With this limitation, I would much prefer to have the head of master branch relatively clean and stable, only advanced by good quality, tested, and self-contained contributions, not by work-in-progress fragments. 3. Large submission does not necessarily equal to behind-door development. Gitlab is cheap. Self-formed herds, even individual ONAP projects, can set up their own git server and apply all fancy workflows there, then make the whole contribution to ONAP when they are ready. I actually believe this is more community based development. 4. Studies show that there is a loglog relationship between number of submissions and submission sizes for open source commits. Large submissions do happen, but are rare. We only see more of those now because we are pre-R1, many contributions are ports from OpenO, OpenECOMP, or member companies own code. As ONAP grows, more and more code will be developed along with ONAP, and I have no doubt we will see fewer and fewer mega submissions. 1. With all the above said, I do wish that it would be great if the 36 hour review deadline rule can be flexible based on submission size. Thank you for reading. Lusheng ONAP DCAE From: <onap-tsc-boun...@lists.onap.org> on behalf of Stephen Terrill <stephen.terr...@ericsson.com> Date: Thursday, August 3, 2017 at 1:42 PM To: "onap-tsc@lists.onap.org" <onap-tsc@lists.onap.org> Subject: Re: [onap-tsc] Enforcing an "Upstream first" approach to ONAP Hi, It great that David highlighted this need, just a few thoughts from my perspective: * I read that there is general agreement about going for upstream first. Maybe its not about introducing something new actually. When looking at our Wiki, we already have guidance: https://wiki.onap.org/display/DW/Continuous+Integration<https://urldefense.proofpoint.com/v2/url?u=https-3A__wiki.onap.org_display_DW_Continuous-2BIntegration&d=DwMFAg&c=LFYZ-o9_HUMeMTSQicvjIg&r=qiKb3LQBLje9oy8MjG3MEx1ia4mIocif7RoUY8WQWV8&m=t5dp25cmMjv6cRum8ZeBjkB0d-1VdJqF7AjC3yZ0_Jw&s=u7dbYQ5C7s9xVea7VKhL_KkZI46vTT-t-P3s-9V9qoQ&e=> * We know that the projects are going through a formation and migration and merging, and I can respect the effort that entails. Maybe its enough to highlight this for now that after the “migration/formation” that this is not a great practice. Accepting this may occur during the formation phase and is not a desired habbit, I wonder whether we could simple ask the PTLs when they will be ready to move to continues upstream mode of operation? Leave the projects to define when it makes sense to them in their particular situation. * Anything to do with code styling, best practices, etc. I agree its great to have a desired aligned view. However lets involve the designers and projects, enforcing from “external” may not have traction. My suggestion would be to involve them and have TSC approval based on PTL approval first. Best Regards, Steve. From: onap-tsc-boun...@lists.onap.org [mailto:onap-tsc-boun...@lists.onap.org] On Behalf Of GILBERT, MAZIN E (MAZIN E) Sent: 03 August 2017 18:33 To: Yunxia Chen <helen.c...@huawei.com> Cc: eric.deb...@orange.com; onap-tsc@lists.onap.org Subject: Re: [onap-tsc] Enforcing an "Upstream first" approach to ONAP Thank you Helen. Appreciate the leadership. Mazin Sent through AT&T's fastest network On Aug 3, 2017, at 7:00 PM, Yunxia Chen <helen.c...@huawei.com<mailto:helen.c...@huawei.com>> wrote: Hi, Mazin, The Integration team will take this one as an action item: we will define a list, (there’s other best code practice as well, such as code review etc.). with an implementation plan since we may not able to do everything in R1, and get TSC’s approval. Regards, Helen Chen From: <onap-tsc-boun...@lists.onap.org<mailto:onap-tsc-boun...@lists.onap.org>> on behalf of "GILBERT, MAZIN E (MAZIN E)" <ma...@research.att.com<mailto:ma...@research.att.com>> Date: Thursday, August 3, 2017 at 8:31 AM To: Eric Debeau <eric.deb...@orange.com<mailto:eric.deb...@orange.com>> Cc: onap-tsc <onap-tsc@lists.onap.org<mailto:onap-tsc@lists.onap.org>> Subject: Re: [onap-tsc] Enforcing an "Upstream first" approach to ONAP I am ok if the integration team wants to take the task of defining best code practices and guidelines on significant code insertion. I agree with Ash that code that can not be reviewed should not be committed blindly. But we need to form guidelines so we are all following the same practices. Even the concept of significant code is not defined. This is not a trivial effort and I do not want the integration team to be distracted from R1, but I am open for suggestions from the TSC. Let's have the discussion via email. I would also appreciate links to successful code best practices and enforcements adopted by other forums. Mazin Sent through AT&T's fastest network On Aug 3, 2017, at 11:35 AM, "eric.deb...@orange.com<mailto:eric.deb...@orange.com>" <eric.deb...@orange.com<mailto:eric.deb...@orange.com>> wrote: +1 for raising this topic We should avoid 10k LOC patches because it is very complex to review and is not aligned with an open source spirit. I do not believe that we should create another subcommitte to handle such issue. The integration project may be able to detect such behavior and alerts the PTL and TSC I also agree with Dhananjay => We spend too much effort on functional aspects for R1. There is still some issues to setup a full ONAP platform on Vanilla OpenStack. Best regards Eric _________________________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ ONAP-TSC mailing list ONAP-TSC@lists.onap.org<mailto:ONAP-TSC@lists.onap.org> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.onap.org_mailman_listinfo_onap-2Dtsc&d=DwICAg&c=LFYZ-o9_HUMeMTSQicvjIg&r=IKSC5mg8GeOiSar1dax3GQ&m=rSkZu5gsdRT8HwFjzhd2QXyGHqPCRZUGh5Z88VRAiJs&s=o6YwYnIHiT-aLyFYu-yYBdw3IkZxVziwKVEUvMO7h6c&e= "Confidentiality Warning: This message and any attachments are intended only for the use of the intended recipient(s). are confidential and may be privileged. If you are not the intended recipient. you are hereby notified that any review. re-transmission. conversion to hard copy. copying. circulation or other use of this message and any attachments is strictly prohibited. If you are not the intended recipient. please notify the sender immediately by return email. and delete this message and any attachments from your system. Virus Warning: Although the company has taken reasonable precautions to ensure no viruses are present in this email. The company cannot accept responsibility for any loss or damage arising from the use of this email or attachment."
_______________________________________________ ONAP-TSC mailing list ONAP-TSC@lists.onap.org https://lists.onap.org/mailman/listinfo/onap-tsc
