#1 - yes. we had coding standards before, but they are documented now, which makes for better overall experience for new contributors
#2 - New code should obviously follow standards. I support modifying existing code in separate JIRAs when the existing code is particularly bad and a large change is required to make it acceptable. Otherwise, I like Jarcec suggestion: - Cleanup can be done while working on unrelated patches, provided that the scope of the cleanup is smaller than that of the patch. I.e. - if the actual fix is 3 lines, don't add 100 lines of cleanup. If the actual fix is 100 lines, 3 lines of low-risk cleanup are acceptable. The idea is that when looking at the JIRA and the patch, 90% of the patch should be related to the issue described in the JIRA. Gwen On Mon, Dec 1, 2014 at 7:04 PM, Veena Basavaraj <[email protected]> wrote: > Very well articulated Gwen. > > So the question is > #1. Should we start using code standards? > #2, if we do, what is the best way to enforce it? > > > > > > Best, > *./Vee* > > On Mon, Dec 1, 2014 at 6:39 PM, Gwen Shapira <[email protected]> wrote: > >> Regarding applying the changes: >> >> I think we want to have good balance between few goals: >> 1. Minimize risk of changes by having a well defined scope >> 2. Make reviewing easier >> 3. Ease of cherry-picking >> 4. Ease of incrementally improving code quality >> >> Our current process leans toward the first three goals, but >> discourages small improvements by adding quite a bit of process around >> them. >> >> I'd like to see us a bit more open toward small code improvements that >> do not add significant risk to the project. Spelling of variable >> names, import order, indentation, comment readability, etc. I believe >> it will lead to better code in the long term, without significant >> drawbacks. >> >> Gwen >> >> >> On Mon, Dec 1, 2014 at 6:17 PM, Jarek Jarcec Cecho <[email protected]> >> wrote: >> > Thank you for putting the coding guidelines together Veena. I’ve read >> the page and I don’t see anything concerning. I have to admit that I did >> not read the whole Google guideline so I’m wondering if there are any huge >> differences between current (albeit unwritten) rules and the proposed >> Google guidelines? >> > >> > I’ve added/edited the content a bit, trying to put some small comments >> without changing the semantics of the page. I have following larger points: >> > >> > 1) Line size >> > >> > The “General Rules” section specifies that the “Column limit can be 80 >> or 100 characters”, the subsection “ Column Limit and Line Wrapping” is >> stating only “100” and then suggesting that we could also drop any line >> limits. I would suggest to synchronize those places together as it seems >> quite confusing. >> > >> > I would personally vote to not have any sharp limit for maximum line >> size. We have 80 characters limit in Sqoop 1 and it makes code very hardly >> readable at some places, so I like the Kafka wording “no maximum size, but >> be reasonable”. >> > >> > 2) Applying the changes >> > >> > Considering that we might introduce some changes against current >> (unwritten) rules, I think that we should talk about how we’re going to >> enforce the coding guidelines going forward. This is not a question for new >> files as those should obviously follow them. I’m more interested to discuss >> how we’re going to handle changes in current files. As we are trying to >> keep each patch focusing on given functionality and we’re discouraging >> unrelated changes, I would propose to discourage people to enforce those >> rules in existing files if that would mean to significantly refactor the >> file just for the purpose of being compliant with our code guidelines. >> > >> > This of course doesn’t mean that people should not follow the guidelines >> at all (new content should follow them) or that we’re going to stick to the >> old formatting forever (on large file refactoring it would be OK to apply >> such changes). I’m just worried about introducing a lot of unnecessary >> changes just to keep in sync with coding guidelines. >> > >> > Jarcec >> > >> >> On Nov 30, 2014, at 6:29 PM, Veena Basavaraj <[email protected]> >> wrote: >> >> >> >> Qian, >> >> >> >> I think we both are on the same page. >> >> >> >> I just wrote the guidelines wiki to to come up with the code style. >> >> >> >> One we all vote on the wiki, we can formulate those rules as IDE >> specific >> >> files, like the one you attached for intelliJ >> >> >> >> Hope it clears the confusion now. >> >> >> >> >> >> >> >> >> >> Best, >> >> *./Vee* >> >> >> >> On Sun, Nov 30, 2014 at 6:20 PM, Xu, Qian A <[email protected]> >> wrote: >> >> >> >>> Hi Veena, >> >>> >> >>> A code guideline will definitively help both developers and reviews to >> >>> contribute code better. I'd suggest apply code style to new code >> >>> contribution first. >> >>> >> >>> --Qian (Stanley) Xu >> >>> >> >>> -----Original Message----- >> >>> From: Veena Basavaraj [mailto:[email protected]] >> >>> Sent: Monday, December 01, 2014 10:09 AM >> >>> To: [email protected] >> >>> Subject: Re: Sqoop 2 JAVA coding guidelines >> >>> >> >>> Thanks Qian, I will attach it, do you have any comments on the >> guidelines? >> >>> We should all vote for the wiki guidelines before we attach the >> formatter. >> >>> >> >>> Please send an email to the dev@ list asking permission to edit the >> wiki. >> >>> >> >>> >> >>> >> >>> >> >>> Best, >> >>> *./Vee* >> >>> >> >>> On Sun, Nov 30, 2014 at 5:59 PM, Xu, Qian A <[email protected]> >> wrote: >> >>> >> >>>> Hi there. >> >>>> >> >>>> I don’t have the privilege to add attachment to Wiki , so I've >> >>>> attached code formatter of IntelliJ (v13.1). I hope this helps. >> >>>> >> >>>> --Qian (Stanley) Xu >> >>>> >> >>>> >> >>>> -----Original Message----- >> >>>> From: Gwen Shapira [mailto:[email protected]] >> >>>> Sent: Monday, December 01, 2014 8:46 AM >> >>>> To: [email protected] >> >>>> Subject: Re: Sqoop 2 JAVA coding guidelines >> >>>> >> >>>> I think its a great idea to document the coding standards that already >> >>>> exist for this project. >> >>>> It can be frustrating to new contributors to have to guess their way >> >>>> around our un-written expectations. >> >>>> >> >>>> Gwen >> >>>> >> >>>> On Sun, Nov 30, 2014 at 2:05 PM, Veena Basavaraj >> >>>> <[email protected]> >> >>>> wrote: >> >>>>> Hey all, >> >>>>> >> >>>>> Since a few months I have been contributing to Sqoop2, I do notice a >> >>>>> lack of style guide for sqoop2 code. >> >>>>> >> >>>>> I have started a wiki highlighting things for developers to follow. >> >>>>> >> >>>>> https://cwiki.apache.org/confluence/display/SQOOP/Sqoop+2+Coding+Gui >> >>>>> de lines I prefer the google java style guide to begin with. >> >>>>> >> >>>>> Please add your comments in the comments section on things you would >> >>>>> like to have, so we can iterate over this in the coming few weeks >> >>>>> and settle down to a standard that every one can use in their IDE. I >> >>>>> am happy to create a formatter XML for eclipse as per these rules >> >>>>> and attach it to the wiki. >> >>>>> >> >>>>> >> >>>>> >> >>>>> Best, >> >>>>> *./Vee* >> >>>> >> >>> >> > >>
