[aerogear-dev] Formatting versus code fixes

[Kris Borchers]

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 at apache.org> wrote:

> 
> 
> 
> On Thu, Sep 12, 2013 at 2:44 PM, Lucas Holmquist <lholmqui at redhat.com> wrote:
>> 
>> On Sep 12, 2013, at 8:38 AM, Bruno Oliveira <bruno at 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 at redhat.com>
>> >> September 12, 2013 9:31 AM
>> >> +1
>> >>
>> >>
>> >> _______________________________________________
>> >> aerogear-dev mailing list
>> >> aerogear-dev at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> >> Daniel Bevenius <mailto:daniel.bevenius at gmail.com>
>> >> September 12, 2013 4:13 AM
>> >> #agreed
>> >>
>> >>
>> >>
>> >> _______________________________________________
>> >> aerogear-dev mailing list
>> >> aerogear-dev at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> >> Matthias Wessendorf <mailto:matzew at 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 at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> >
>> > --
>> > abstractj
>> >
>> >
>> > _______________________________________________
>> > aerogear-dev mailing list
>> > aerogear-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> 
>> 
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev at 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20130912/0e8a800f/attachment-0001.html