+1 for #1. On Oct 20, 2015 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 > >
