Mailing-List: contact dev-help@apex.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@apex.incubator.apache.org
Subject: Re: Existing checkstyle violations
To: dev@apex.incubator.apache.org
References: <562684A9.9080003@datatorrent.com>
 <CAKaBFBkp86PUTUi7Kpbrh0UkWtLO5Dg3ucjo83x7veS=7E2X-A@mail.gmail.com>
From: Vlad Rozov <v.rozov@datatorrent.com>
Organization: DataTorrent
Message-ID: <56268682.80401@datatorrent.com>
Date: Tue, 20 Oct 2015 11:22:58 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0)
 Gecko/20100101 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: 
 <CAKaBFBkp86PUTUi7Kpbrh0UkWtLO5Dg3ucjo83x7veS=7E2X-A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

+1 for 3.

On 10/20/15 11:21, Chandni Singh wrote:
> +1 for approach 2
> Preserves git annotation.
>
> On Tue, Oct 20, 2015 at 11:15 AM, Vlad Rozov <v.rozov@datatorrent.com>
> wrote:
>
>> 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
>>
>>