The pre-big bang attributions should still be retrievable by looking at the commit just prior, so, though it is a bit more work, the information is not irretrievably lost.
Ram On Wed, Oct 21, 2015 at 10:04 AM, Pramod Immaneni <[email protected]> wrote: > Are you sure the majority of the formatting changes are number of > whitespaces w.r.t your third point. > > On Wed, Oct 21, 2015 at 10:00 AM, David Yan <[email protected]> wrote: > > > I think this is exactly what we have been doing. > > > > The reasons why I think #1 is good is because: > > > > 1) When you write new code that breaks the style check, it's not obvious > > what specific violation the new code introduces because the code base has > > already some hundred existing violations. I have already suffered from > > that many times since the stylecheck plugin was put in. > > > > 2) It's confusing to newcomers when they see part of the code is in one > > format and part of the code is in another, and it's displeasing to the > > eyes, and it does not promote consistency. > > > > 3) git blame -w ignores whitespaces, and it looks like IntelliJ does that > > too after 11.1.3: https://youtrack.jetbrains.com/issue/IDEA-55075. We > are > > just left with whether curly braces are on the same line for "else", etc. > > But the bottom line is, the meat of the code should still be largely > > attributed to the proper authors. > > > > David > > > > > > > > > > On Wed, Oct 21, 2015 at 9:44 AM, Pramod Immaneni <[email protected] > > > > 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 > > > >>> > > > >>> > > > >>> > > > > > > > > > >
