apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Priyanka Gugale <priya...@datatorrent.com>
Subject Re: Existing checkstyle violations
Date Wed, 21 Oct 2015 06:27:02 GMT
Option #1 looks good though it's bit risky.
And if we choose option #3 we can have  a practice to have two pull
requests, first for formatting changes and other for code changes. This
will be an overhead for developers though and could increase the time to
get code pulled.

-Priyanka

On Wed, Oct 21, 2015 at 11:30 AM, Sandeep Deshmukh <sandeep@datatorrent.com>
wrote:

> +1 for #1.
>
> Regards,
> Sandeep
>
> On Wed, Oct 21, 2015 at 12:38 AM, York, Brennon <
> Brennon.York@capitalone.com
> > wrote:
>
> > +1 for #1. Its hard and ruthless, but I¹d rather rip that bandaid off.
> >
> > On 10/20/15, 11:29 AM, "Ganelin, Ilya" <Ilya.Ganelin@capitalone.com>
> > wrote:
> >
> > >I think it can be a mix. I think folks should be able to submit patches
> > >that are only style fixes (for an entire file) OR fix minor things as
> > >they go. I don't think major stylistic overhauls should be mixed with
> > >logical changes.
> > >
> > >
> > >
> > >Thank you,
> > >Ilya Ganelin
> > >
> > >
> > >
> > >-----Original Message-----
> > >From: Vlad Rozov [v.rozov@datatorrent.com<mailto:
> v.rozov@datatorrent.com
> > >]
> > >Sent: Tuesday, October 20, 2015 02:15 PM Eastern Standard Time
> > >To: dev@apex.incubator.apache.org
> > >Subject: Existing checkstyle violations
> > >
> > >
> > >All,
> > >
> > >We have a large number of existing checkstyle violations and it is
> > >cumbersome to distinguish a newly introduced violation from existing
> > >ones. We need to agree on the process to fix them and there are multiple
> > >approaches how we can do it.
> > >
> > >1. Fix them all in a single commit (one commit for Core and one for
> > >Malhar). Pros: change can easily be distinguished from logical code
> > >changes. Cons: large number of changes in a single commit, hard to
> > >review. Changes and review likely to be done by developers not familiar
> > >with code specifics.
> > >2. Fix as we go. Only change code style violation in modified places.
> > >Pros: limited amount of change. Easy to review. Cons: likely to take
> > >forever. Some part of the code may not be fixed at all.
> > >3. Somewhat combination of 1 & 2. Fix all violations in files affected
> > >by a commit. Pros: changes likely to be done by developers familiar with
> > >the code. Cons: harder to distinguish between logical and style changes
> > >in a single commit.
> > >4. Any other suggestions?
> > >
> > >Independently of the approach selected, we should not allow commits
> > >where entire file is modified due to style modifications. Such file(s)
> > >needs to be fixed and committed using Malhar CI.
> > >
> > >Thank you,
> > >
> > >Vlad
> > >
> > >
> > >________________________________________________________
> > >
> > >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.
> > >________________________________________________________
> > >
> > >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.
> >
> > ________________________________________________________
> >
> > 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.
> >
> > ________________________________________________________
> >
> > 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.
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message