sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Veena Basavaraj <vbasava...@cloudera.com>
Subject Re: Sqoop 2 JAVA coding guidelines
Date Tue, 02 Dec 2014 16:18:45 GMT
Let me give a more concrete example

#1, if we start using the standards uniformly from now, this means we use a
IDE formatter that adheres to these rules.

So say I am modifying a existing file, I add some 5 new classes and needs
imports, I usually tend to use the eclipse shortcut
<http://www.rossenstoyanchev.org/write/prog/eclipse/eclipse3.html> based on
the formatter to do it, it will override and use the new order and there
are bound to be changes. So according to you, we just append new imports
manually to this file, so we dont create more than 3 lines of changes?

I fail to understand why imports re-ordered makes it hard to read a change
even in git blame. It is very isolated change in the top of the file and
can be easily ignored since we know we will be changing it going forward to
keep one standard.

As annoying as it sounds to have this rule, I am happy to document this
since everyone contributing to sqoop will be aware of this.





Best,
*./Vee*

On Tue, 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message