I prefer the simplicity of 1 -- pay the price once and the issue is solved forever (I know, I'm an optimist).
Ram 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 > >
