+1 for 3. The committer should be responsible to catch extreme cases. For example, we don't want formatting of an entire file mixed with a few lines net change in the same commit.
On Tue, Oct 20, 2015 at 11:22 AM, Vlad Rozov <[email protected]> wrote: > +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 <[email protected]> >> 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 >>> >>> >>> >
