[aerogear-dev] Formatting versus code fixes

Bruno Oliveira bruno at abstractj.org
Thu Sep 12 09:00:41 EDT 2013 

From my humble perspective there are 2 alternatives:

- Be conscious about what you are doing that means (what I'm sending on
this PR, instead of git commit -a)
- Add a baby sitting service like checkstyle and make the build fail :)

> Matthias Wessendorf <mailto:matzew at apache.org>
> September 12, 2013 9:49 AM
>
>
>
> On Thu, Sep 12, 2013 at 2:44 PM, Lucas Holmquist <lholmqui at redhat.com
> <mailto:lholmqui at redhat.com>> wrote:
>
>
>     On Sep 12, 2013, at 8:38 AM, Bruno Oliveira <bruno at abstractj.org
>     <mailto: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
>     <mailto:lholmqui at redhat.com>>
>     >> September 12, 2013 9:31 AM
>     >> +1
>     >>
>     >>
>     >> _______________________________________________
>     >> aerogear-dev mailing list
>     >> aerogear-dev at lists.jboss.org <mailto:aerogear-dev at lists.jboss.org>
>     >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>     >> Daniel Bevenius <mailto:daniel.bevenius at gmail.com
>     <mailto:daniel.bevenius at gmail.com>>
>     >> September 12, 2013 4:13 AM
>     >> #agreed
>     >>
>     >>
>     >>
>     >> _______________________________________________
>     >> aerogear-dev mailing list
>     >> aerogear-dev at lists.jboss.org <mailto:aerogear-dev at lists.jboss.org>
>     >> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>     >> Matthias Wessendorf <mailto:matzew at apache.org
>     <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 <mailto: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 <mailto:aerogear-dev at lists.jboss.org>
>     > https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
>     _______________________________________________
>     aerogear-dev mailing list
>     aerogear-dev at lists.jboss.org <mailto: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
> Lucas Holmquist <mailto:lholmqui at redhat.com>
> September 12, 2013 9:44 AM
> 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.
>
>
> 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
> 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
Url : http://lists.jboss.org/pipermail/aerogear-dev/attachments/20130912/822a5842/attachment.bin

More information about the aerogear-dev mailing list