Yes, I think the misplaced "else" should be fixed. Ram
On Wed, Oct 21, 2015 at 10:00 AM, Vlad Rozov <[email protected]> wrote: > This is a variation of option #2 with slightly extended scope. My concern > with this approach is that it is vague. In the following case > > if () { > if () { > -> logical changes here > } else { > } > } > else { ==? is it OK to fix code style here? > > > } > > On 10/21/15 09:44, Pramod Immaneni wrote: > >> I would go with a modification of 3, you could call it 4. Instead of >> re-formatting the entire file that is touched, re-format the code that is >> in and around the modified code. Leave that determination to developer >> discretion. For example if you are making changes to the if block, fix the >> else as well or fix that method if the code changes you are making to the >> method are substantial. I would not like the entire file to be changed as >> that changes attribution without changing the logic and makes it difficult >> to find out whom to talk to when trying to inquire about a piece of code. >> >> Thanks >> >> On Wed, Oct 21, 2015 at 9:06 AM, Vlad Rozov <[email protected]> >> wrote: >> >> I filed https://malhar.atlassian.net/browse/APEX-207 to track the issue. >>> Please use it as parent for all check style related JIRAs/fixes. Anyone >>> to >>> volunteer to work on #1 approach? >>> >>> Thank you, >>> >>> Vlad >>> >>> >>> On 10/21/15 00:28, David Yan wrote: >>> >>> +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 >>>>> >>>>> >>>>> >>>>> >
