Hi Pam, Thanks for the response.
“One thing I would like to share is that for my project, we agreed as committers that we are going to require going forward that any code submission be preceded or followed by checkstyle fixes. That is, if they are changing a file, then they must submit it first with formatting done. Then submit the change they trying to introduce. Or vice versa fixing any newly introduced sonar problems.” This is similar to my ask. I’m not partial to any particular method, as long as the end result is a cleaner, well styled codebase. In other open source projects I’ve worked on as committer (OpenDaylight, OPNFV) we enforced the style rules as part of the review process. You could also take the route you suggest. My ask isn’t to require anyone to do this, (although last release there was some talk that style would be enforced in the next release per TSC or something along those lines) but that we (the SO project PTL + Committers) proactively push our contributors and each other to do better. I think SO has done pretty well with junit and sonar. We need a bit more work in useful CSIT. But style is something of a nit for me. I love seeing clean and well formatted code, that way I can easily focus on substance. We do a middling job at that and I’d like us (SO, but also ONAP) to work on improving. My suggestion is one way to start. Thanks, Marcus Williams IRC, Twitter, etc. @ mgkwill Intel Corp. From: [email protected] [mailto:[email protected]] On Behalf Of Pamela Dragosh Sent: Thursday, August 2, 2018 6:07 PM To: [email protected] Subject: Re: [onap-discuss] [SO] Committer's enforcing ONAP Code Standards? Marcus, I am on board with this. I understand folks think its tedious, but having clean well-formatted code really helps with the reviewers. I have been asking anyone who wants to get started to contribute for Policy to start with sonar and checkstyle issues. They are an easy way to get familiar not only with the codebase, but also ONAP code review process. I have had some folks perform awesome with this, while others faded. One thing I would like to share is that for my project, we agreed as committers that we are going to require going forward that any code submission be preceded or followed by checkstyle fixes. That is, if they are changing a file, then they must submit it first with formatting done. Then submit the change they trying to introduce. Or vice versa fixing any newly introduced sonar problems. Sometimes they can be done in the submission itself, but we found as reviewers that it was too difficult to sort out the actual changes vs checkstyle formatting. We shall see how that goes. I don’t think we should force any requirements right now. We have too much a burden with the workload that we have and laying down more requirements as a release goes along tends to upset us PTL’s. But perhaps over the next few releases we can buckle down on checkstyle, sonar and Junit code coverage. These things take time both doing them and reviewing them. Thanks, Pam Dragosh Policy PTL From: <[email protected]<mailto:[email protected]>> on behalf of Marcus G K Williams <[email protected]<mailto:[email protected]>> Reply-To: "[email protected]<mailto:[email protected]>" <[email protected]<mailto:[email protected]>>, "[email protected]<mailto:[email protected]>" <[email protected]<mailto:[email protected]>> Date: Thursday, August 2, 2018 at 7:59 PM To: onap-discuss <[email protected]<mailto:[email protected]>> Subject: [onap-discuss] [SO] Committer's enforcing ONAP Code Standards? Hi SO Committers, PTL and community, I think we have an opportunity to be an earlier adopter in the community. I’d like to propose that we work on conforming our codebase to ONAP code standards: https://wiki.onap.org/display/DW/Java+code+style<https://urldefense.proofpoint.com/v2/url?u=https-3A__wiki.onap.org_display_DW_Java-2Bcode-2Bstyle&d=DwMFAg&c=LFYZ-o9_HUMeMTSQicvjIg&r=jwTiArcEj6aUX0HjV0M3dT12gUtk7rC07xpgpVZkS_4&m=rJ9UalTmO7Btt9-Ksm2omPzpxDDcpovGdhaSqXYmGnE&s=7TKiF5TEtByqGTULykn7So1CNkzyRAh6B5PQT2qrM3E&e=> Generally this entails 4 spaces in place of tabs, 120 character line limit and google style guidelines, which largely follow generally accepted Java Style. Our codebase does not currently conform to these standards. Further some of the style issues result in a very large amount of warnings in build logs (last time I counted it was 10s of thousands), which increase build time and space used by Jenkins/LF for each build. We should also make sure we don’t have extraneous spaces (gerrit highlights those in red and having them in open source code is generally frowned upon by any self-respecting open source engineer). Does anyone have any problem with working to make the code conform to style guidelines? Can we agree to -1 changes that do not conform until the submitter conforms the patch to style guidelines? As a corollary I’d suggest we enforce commit message guidelines as well: https://wiki.onap.org/display/DW/Commit+Messages<https://urldefense.proofpoint.com/v2/url?u=https-3A__wiki.onap.org_display_DW_Commit-2BMessages&d=DwMFAg&c=LFYZ-o9_HUMeMTSQicvjIg&r=jwTiArcEj6aUX0HjV0M3dT12gUtk7rC07xpgpVZkS_4&m=rJ9UalTmO7Btt9-Ksm2omPzpxDDcpovGdhaSqXYmGnE&s=W1EMRLgeXJ5IfJDnLqy0aGRBI57GqFI2XqJ8gGgaoBI&e=> Thanks, Marcus Williams IRC, Twitter, etc. @ mgkwill Intel Corp. -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#11659): https://lists.onap.org/g/onap-discuss/message/11659 Mute This Topic: https://lists.onap.org/mt/24149869/21656 Group Owner: [email protected] Unsubscribe: https://lists.onap.org/g/onap-discuss/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
