apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vlad Rozov <v.ro...@datatorrent.com>
Subject Re: Existing checkstyle violations
Date Tue, 20 Oct 2015 18:22:58 GMT
+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
>>
>>


Mime
View raw message