sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Jarcec Cecho <jar...@apache.org>
Subject Re: Sqoop 2 JAVA coding guidelines
Date Wed, 03 Dec 2014 01:45:34 GMT
I wanted to follow up on this particular email because I really like how Gwen worded the summary
for #2 - that is indeed exactly what I had on my mind :) If JIRA summary says “I’m doing
X”, I would expect that 90%+ of the path will cover “X”.

Jarcec

> On Dec 2, 2014, at 7:57 AM, Gwen Shapira <gshapira@cloudera.com> wrote:
> 
> #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 <vbasavaraj@cloudera.com> 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 <gshapira@cloudera.com> 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 <jarcec@apache.org>
>>> 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 <vbasavaraj@cloudera.com>
>>> 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 <qian.a.xu@intel.com>
>>> 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:vbasavaraj@cloudera.com]
>>>>>> Sent: Monday, December 01, 2014 10:09 AM
>>>>>> To: dev@sqoop.apache.org
>>>>>> 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 <qian.a.xu@intel.com>
>>> 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:gshapira@cloudera.com]
>>>>>>> Sent: Monday, December 01, 2014 8:46 AM
>>>>>>> To: dev@sqoop.apache.org
>>>>>>> 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
>>>>>>> <vbasavaraj@cloudera.com>
>>>>>>> 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*
>>>>>>> 
>>>>>> 
>>>> 
>>> 


Mime
View raw message