Mailing-List: contact dev-help@apex.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@apex.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAKJfLDOSLe7nAtXjcWMT4PwHdhUjexq0W2KRq4DkYfGhri6mfA@mail.gmail.com>
References: 
 <CAKaBFB=Xqq5ejZu0ojomTieReNJ5ujZTQ0F12ecGGnDfYa=dMg@mail.gmail.com>
	<95895BF14D8F9440BE1D6043012551A6053DC53F@KDCPEXCMB13.cof.ds.capitalone.com>
	<CADS+zCsYMn43HgdkbsBbSnX=yDG02dNX85nyydkoLo9BqZcjCw@mail.gmail.com>
	<CAJ484BjMQH1MPdVbmPo3UXdipuD3z=guH4bkisbXE6chSWhqSg@mail.gmail.com>
	<CAKJfLDOSLe7nAtXjcWMT4PwHdhUjexq0W2KRq4DkYfGhri6mfA@mail.gmail.com>
Date: Tue, 17 Nov 2015 18:36:57 -0800
Message-ID: 
 <CAKaBFB=2Br36s1psEXPMTwFFGbvKunGzqGQRSWo98Tej81cNpg@mail.gmail.com>
Subject: Re: Extent of attention to code style in code reviews
From: Chandni Singh <chandni@datatorrent.com>
To: dev@apex.incubator.apache.org
Content-Type: multipart/alternative; boundary=001a11420af41fd5ac0524c784ef

--001a11420af41fd5ac0524c784ef
Content-Type: text/plain; charset=UTF-8

Hey David,

In my pull request, I was fixing checkstyle violations. The lines were
broken as per my IDE settings. In order to satisfy everyone's comment I had
to go and tweak the settings however I don't think we achieve anything by
that. I checked Netbeans and eclipse and they don't have as many options as
IntelliJ. So then what does the developer do? Manually go and break the
line where it is desired by the reviewer?

The pull request I created is closed and I think individual owners need to
go and fix the style. However I am concerned if this is the level of review
done on pull requests of a feature.

I agree with Tim completely. We have more important items to work on.

Thanks,
Chandni

On Tue, Nov 17, 2015 at 6:26 PM, Thomas Weise <thomas@datatorrent.com>
wrote:

> +1 for not boiling the ocean. There are more important items on the
> roadmap.
>
> --
> sent from mobile
> On Nov 17, 2015 6:21 PM, "Munagala Ramanath" <ram@datatorrent.com> wrote:
>
> > Agree completely. Let's go with what check style can check and not
> > worry about the
> > rest.
> >
> > Ram
> >
> > On Tue, Nov 17, 2015 at 6:02 PM, Timothy Farkas <tim@datatorrent.com>
> > wrote:
> > > I think we already have a consistent style adopted and it is
> sufficient.
> > > There are bigger priorities for the project than enforcing the optimal
> > way
> > > to format wrapped lines or other similar things. Frankly we should be
> > > focusing on adding more unit tests, fixing bugs, and fixing features
> that
> > > aren't fault tolerant (property setting).
> > >
> > > Just my opinion,
> > > Tim
> > >
> > > On Tue, Nov 17, 2015 at 5:45 PM, Ganelin, Ilya <
> > Ilya.Ganelin@capitalone.com>
> > > wrote:
> > >
> > >> As long as there is consistency between code style and check style
> > that's
> > >> all that should matter. If consistency is impossible, then the check
> > style
> > >> shouldn't exist imo.
> > >>
> > >>
> > >>
> > >> Thank you,
> > >> Ilya Ganelin
> > >>
> > >>
> > >>
> > >> -----Original Message-----
> > >> From: Chandni Singh [chandni@datatorrent.com<mailto:
> > >> chandni@datatorrent.com>]
> > >> Sent: Tuesday, November 17, 2015 08:24 PM Eastern Standard Time
> > >> To: dev@apex.incubator.apache.org
> > >> Subject: Extent of attention to code style in code reviews
> > >>
> > >>
> > >> Since we are getting religious about code styles and concerns are
> being
> > >> raised about wrapping lines in a better way to improve readability, I
> > think
> > >> this is one place we can learn from some other apache projects.
> > >>
> > >> https://flink.apache.org/contribute-code.html#code-style
> > >>
> > >> https://maven.apache.org/developers/conventions/code.html
> > >>
> > >> https://wiki.apache.org/cassandra/CodeStyle
> > >>
> > >> IMO none of these projects are enforcing style to the extent we have
> > >> adopted.
> > >> I think if we start focussing on style to this extent, that is,  where
> > to
> > >> break a line while wrapping, then our review process will result in
> more
> > >> frustration. I think this doesn't help the community to grow.
> > >>
> > >> Thanks,
> > >> Chandni
> > >> ________________________________________________________
> > >>
> > >> The information contained in this e-mail is confidential and/or
> > >> proprietary to Capital One and/or its affiliates and may only be used
> > >> solely in performance of work or services for Capital One. The
> > information
> > >> transmitted herewith is intended only for use by the individual or
> > entity
> > >> to which it is addressed. If the reader of this message is not the
> > intended
> > >> recipient, you are hereby notified that any review, retransmission,
> > >> dissemination, distribution, copying or other use of, or taking of any
> > >> action in reliance upon this information is strictly prohibited. If
> you
> > >> have received this communication in error, please contact the sender
> and
> > >> delete the material from your computer.
> > >>
> >
>

--001a11420af41fd5ac0524c784ef--