IMO, formatting changes should always be in their own commit. I agree that you should not add formatting fixes in multiple files along with a real code change in the same PR. That makes it to hard to review. If its in the same file though, I don't mind formatting commits in the same PR and even in the history but if people don't want that noise in history, the person that merges the PR can always squash before they merge.

On Sep 12, 2013, at 8:43, Matthias Wessendorf <matzew@apache.org> wrote:




On Thu, Sep 12, 2013 at 2:44 PM, Lucas Holmquist <lholmqui@redhat.com> wrote:

On Sep 12, 2013, at 8:38 AM, Bruno Oliveira <bruno@abstractj.org> wrote:

> Guys, just run mvn java-formatter:formatbefore commit, we have it
> enabled at POM parent as far as I know.

yup,  as long as we are doing it every time we do a PR.

The problem with adding a "fix formatting" commit in the _same_ PR than "fix a real problem" is: Too much noise.

IMO it's just odd if 1000 files get formatted in one PR, which actually is intended to include a one line (java) fix

-M
 


but when it's on a mature code base that hasn't had it before and mixed with other changes, then things get hairy

>
>> Lucas Holmquist <mailto:lholmqui@redhat.com>
>> September 12, 2013 9:31 AM
>> +1
>>
>>
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> Daniel Bevenius <mailto:daniel.bevenius@gmail.com>
>> September 12, 2013 4:13 AM
>> #agreed
>>
>>
>>
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> Matthias Wessendorf <mailto:matzew@apache.org>
>> September 12, 2013 4:00 AM
>> Hi,
>>
>> it's nice when we like to format code, however mixing that with actual
>> fixes, makes it VERY hard to review the real change....
>>
>> Please let's not mix these things; It's IMO annoying going over 100
>> files, when the real fix involves just 2 or 3 files....
>>
>> -M
>>
>> --
>> Matthias Wessendorf
>>
>> blog: http://matthiaswessendorf.wordpress.com/
>> sessions: http://www.slideshare.net/mwessendorf
>> twitter: http://twitter.com/mwessendorf
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
> --
> abstractj
>
>
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev


_______________________________________________
aerogear-dev mailing list
aerogear-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--
Matthias Wessendorf

blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf
_______________________________________________
aerogear-dev mailing list
aerogear-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev