You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Andrew Grieve <ag...@chromium.org> on 2012/08/27 16:33:04 UTC

Code Prettifiers

>From the threads in the past on this subject, it seems like:

-Style is an opinion, but:
-A consistent style would be nice even if it's not everyone's preference
-We don't want to shy away contributors by annoying them with style concerns

I think we could get some value out of using a command-line code prettifier
and shared git hooks. The flow I'm envisioning here is:

$ git commit ...
// git hooks complain about your style and tell you to run fix_style
$ bin/fix_style
$ git diff // Shows you what the reformatter did to you.
$ git add ...
$ git commit // passes the style check.


So:
1. We should never have to complain about style when looking at pull
requests
2. We should get a consistent style
3. In the least, I'd like to see consistent tabs v spaces & no trailing ws.

One thing we may have to do is ask a contributor to enable the git hooks.
AFAIK there is no way to enable shared git hooks for a project by default,
so what we'd have is a directory called "hooks" and you'd need to create a
symlink for it into your .git/ directory.

If no one objects, then I'll pursue this today/tomorrow. My plan is to
start with whitespace (sed or awk), get that working, and then see what
x-platform open-source formatters exist out there.

Andrew

Re: Code Prettifiers

Posted by Shazron <sh...@gmail.com>.
iOS actually has an issue in there, filed by me in May:
https://issues.apache.org/jira/browse/CB-625

re: the uncrustify command line tool (GPL)
https://github.com/bengardner/uncrustify
http://uncrustify.sourceforge.net/ pretty much cross-platform

On Mon, Aug 27, 2012 at 11:07 AM, Becky Gibson <gi...@gmail.com> wrote:
> So, as a committer whose code isn't always as clean as could be, I'm all in
> favor of tools that help me to fix my bad habits. I like the hint feature
> of cordova-js.    I'll at least go back to the style guide and make sure my
> xcode settings are correct as I recently re-installed.  However, if you
> work on more than one project (open source or otherwise) sometimes it is
> hard to keep all of the coding "rules" straight between the different
> groups.
>
> -becky
>
>
> On Mon, Aug 27, 2012 at 1:23 PM, Jesse <pu...@gmail.com> wrote:
>
>> Focus on being a contributor and not an arbiter.
>>
>> If these problems do indeed become problems, then we will deal with them,
>> however, I don't think we have issues currently, and even if we do, they
>> are minor IMHO.
>>
>> + everything that Michael said, although, I would not shame anyone, and
>> would simply weigh the value of the contribution with the work required to
>> appease critical peers, before deciding to pull or push back.
>>
>>
>>
>>
>>
>> On Mon, Aug 27, 2012 at 10:12 AM, Patrick Mueller <pm...@gmail.com>
>> wrote:
>>
>> > On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve <agrieve@chromium.org
>> > >wrote:
>> >
>> > > ...
>> > >
>> > > One thing we may have to do is ask a contributor to enable the git
>> hooks.
>> > > AFAIK there is no way to enable shared git hooks for a project by
>> > default,
>> > > so what we'd have is a directory called "hooks" and you'd need to
>> create
>> > a
>> > > symlink for it into your .git/ directory.
>> > >
>> >
>> > Another option is to do this in some sort of continuous build
>> environment.
>> >  Where would the "style enforcer" post it's messages though?  Open bugs?
>> >
>> > If no one objects, then I'll pursue this today/tomorrow. My plan is to
>> > > start with whitespace (sed or awk), get that working, and then see what
>> > > x-platform open-source formatters exist out there.
>> > >
>> >
>> > Presumably these hooks need to be portable to *nix and Windows (where mac
>> > == *nix).  Could we do them in node, without having to shell out to
>> > command-line tools?  Or maybe have the hooks be js that shell out to
>> > cmd-line tools you'd need to have installed.
>> >
>> > --
>> > Patrick Mueller
>> > http://muellerware.org
>> >
>>
>>
>>
>> --
>> @purplecabbage
>> risingj.com
>>

Re: Code Prettifiers

Posted by Becky Gibson <gi...@gmail.com>.
So, as a committer whose code isn't always as clean as could be, I'm all in
favor of tools that help me to fix my bad habits. I like the hint feature
of cordova-js.    I'll at least go back to the style guide and make sure my
xcode settings are correct as I recently re-installed.  However, if you
work on more than one project (open source or otherwise) sometimes it is
hard to keep all of the coding "rules" straight between the different
groups.

-becky


On Mon, Aug 27, 2012 at 1:23 PM, Jesse <pu...@gmail.com> wrote:

> Focus on being a contributor and not an arbiter.
>
> If these problems do indeed become problems, then we will deal with them,
> however, I don't think we have issues currently, and even if we do, they
> are minor IMHO.
>
> + everything that Michael said, although, I would not shame anyone, and
> would simply weigh the value of the contribution with the work required to
> appease critical peers, before deciding to pull or push back.
>
>
>
>
>
> On Mon, Aug 27, 2012 at 10:12 AM, Patrick Mueller <pm...@gmail.com>
> wrote:
>
> > On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve <agrieve@chromium.org
> > >wrote:
> >
> > > ...
> > >
> > > One thing we may have to do is ask a contributor to enable the git
> hooks.
> > > AFAIK there is no way to enable shared git hooks for a project by
> > default,
> > > so what we'd have is a directory called "hooks" and you'd need to
> create
> > a
> > > symlink for it into your .git/ directory.
> > >
> >
> > Another option is to do this in some sort of continuous build
> environment.
> >  Where would the "style enforcer" post it's messages though?  Open bugs?
> >
> > If no one objects, then I'll pursue this today/tomorrow. My plan is to
> > > start with whitespace (sed or awk), get that working, and then see what
> > > x-platform open-source formatters exist out there.
> > >
> >
> > Presumably these hooks need to be portable to *nix and Windows (where mac
> > == *nix).  Could we do them in node, without having to shell out to
> > command-line tools?  Or maybe have the hooks be js that shell out to
> > cmd-line tools you'd need to have installed.
> >
> > --
> > Patrick Mueller
> > http://muellerware.org
> >
>
>
>
> --
> @purplecabbage
> risingj.com
>

Re: Code Prettifiers

Posted by Jesse <pu...@gmail.com>.
Focus on being a contributor and not an arbiter.

If these problems do indeed become problems, then we will deal with them,
however, I don't think we have issues currently, and even if we do, they
are minor IMHO.

+ everything that Michael said, although, I would not shame anyone, and
would simply weigh the value of the contribution with the work required to
appease critical peers, before deciding to pull or push back.





On Mon, Aug 27, 2012 at 10:12 AM, Patrick Mueller <pm...@gmail.com> wrote:

> On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve <agrieve@chromium.org
> >wrote:
>
> > ...
> >
> > One thing we may have to do is ask a contributor to enable the git hooks.
> > AFAIK there is no way to enable shared git hooks for a project by
> default,
> > so what we'd have is a directory called "hooks" and you'd need to create
> a
> > symlink for it into your .git/ directory.
> >
>
> Another option is to do this in some sort of continuous build environment.
>  Where would the "style enforcer" post it's messages though?  Open bugs?
>
> If no one objects, then I'll pursue this today/tomorrow. My plan is to
> > start with whitespace (sed or awk), get that working, and then see what
> > x-platform open-source formatters exist out there.
> >
>
> Presumably these hooks need to be portable to *nix and Windows (where mac
> == *nix).  Could we do them in node, without having to shell out to
> command-line tools?  Or maybe have the hooks be js that shell out to
> cmd-line tools you'd need to have installed.
>
> --
> Patrick Mueller
> http://muellerware.org
>



-- 
@purplecabbage
risingj.com

Re: Code Prettifiers

Posted by Patrick Mueller <pm...@gmail.com>.
On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve <ag...@chromium.org>wrote:

> ...
>
> One thing we may have to do is ask a contributor to enable the git hooks.
> AFAIK there is no way to enable shared git hooks for a project by default,
> so what we'd have is a directory called "hooks" and you'd need to create a
> symlink for it into your .git/ directory.
>

Another option is to do this in some sort of continuous build environment.
 Where would the "style enforcer" post it's messages though?  Open bugs?

If no one objects, then I'll pursue this today/tomorrow. My plan is to
> start with whitespace (sed or awk), get that working, and then see what
> x-platform open-source formatters exist out there.
>

Presumably these hooks need to be portable to *nix and Windows (where mac
== *nix).  Could we do them in node, without having to shell out to
command-line tools?  Or maybe have the hooks be js that shell out to
cmd-line tools you'd need to have installed.

-- 
Patrick Mueller
http://muellerware.org

Re: Code Prettifiers

Posted by Michael Brooks <mi...@michaelbrooks.ca>.
Personally, I'm not a fan of the shared git hooks suggestion. I would be
all for them, if they ran by default without installation. However, I
cannot see the average user reading the documentation to properly install
the hooks, and I don't want git hooks to become an contribution barrier.

I would rather:
  - lead by example with clean code [1]
  - use common/bundled code syntax checkers (e.g. JSHint [2][3], Eclipse's
project formatting XML [4], etc)
  - deny pull requests that are poorly formatted and educate the
contributor on what needs to be corrected
  - publicly shame committers (in a nice manner) who consistently push code
that breaks the file's (lenient) code conventions (e.g. spaces/tabs,
trailing whitespace, braces on the proper line, etc)

[1] http://wiki.apache.org/cordova/StyleGuide
[2] https://github.com/apache/incubator-cordova-js/blob/master/Jakefile#L60
[3] https://github.com/apache/incubator-cordova-js/blob/master/.jshintrc
[4] https://github.com/android/platform_development/tree/master/ide/eclipse

On Mon, Aug 27, 2012 at 8:19 AM, Filip Maj <fi...@adobe.com> wrote:

> Just some extra stuff on the topic:
>
> - There is a style guide on the wiki (although it _does_ need to be filled
> out): http://wiki.apache.org/cordova/StyleGuide
> - cordova-js already has something like this (jshint via a "hint" task [1]
> with config at [2]) but it's hooked in at build-time instead of when
> committing.
>
> [1]
> https://github.com/apache/incubator-cordova-js/blob/master/Jakefile#L60
> [2] https://github.com/apache/incubator-cordova-js/blob/master/.jshintrc
>
> On 8/27/12 7:53 AM, "Simon MacDonald" <si...@gmail.com> wrote:
>
> >For those of us on Android using Eclipse we could standardize on:
> >
> >https://github.com/android/platform_development/tree/master/ide/eclipse
> >
> >Simon Mac Donald
> >http://hi.im/simonmacdonald
> >
> >
> >On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve
> ><ag...@chromium.org>wrote:
> >
> >> From the threads in the past on this subject, it seems like:
> >>
> >> -Style is an opinion, but:
> >> -A consistent style would be nice even if it's not everyone's preference
> >> -We don't want to shy away contributors by annoying them with style
> >> concerns
> >>
> >> I think we could get some value out of using a command-line code
> >>prettifier
> >> and shared git hooks. The flow I'm envisioning here is:
> >>
> >> $ git commit ...
> >> // git hooks complain about your style and tell you to run fix_style
> >> $ bin/fix_style
> >> $ git diff // Shows you what the reformatter did to you.
> >> $ git add ...
> >> $ git commit // passes the style check.
> >>
> >>
> >> So:
> >> 1. We should never have to complain about style when looking at pull
> >> requests
> >> 2. We should get a consistent style
> >> 3. In the least, I'd like to see consistent tabs v spaces & no trailing
> >>ws.
> >>
> >> One thing we may have to do is ask a contributor to enable the git
> >>hooks.
> >> AFAIK there is no way to enable shared git hooks for a project by
> >>default,
> >> so what we'd have is a directory called "hooks" and you'd need to
> >>create a
> >> symlink for it into your .git/ directory.
> >>
> >> If no one objects, then I'll pursue this today/tomorrow. My plan is to
> >> start with whitespace (sed or awk), get that working, and then see what
> >> x-platform open-source formatters exist out there.
> >>
> >> Andrew
> >>
>
>

Re: Code Prettifiers

Posted by Filip Maj <fi...@adobe.com>.
Just some extra stuff on the topic:

- There is a style guide on the wiki (although it _does_ need to be filled
out): http://wiki.apache.org/cordova/StyleGuide
- cordova-js already has something like this (jshint via a "hint" task [1]
with config at [2]) but it's hooked in at build-time instead of when
committing.

[1] https://github.com/apache/incubator-cordova-js/blob/master/Jakefile#L60
[2] https://github.com/apache/incubator-cordova-js/blob/master/.jshintrc

On 8/27/12 7:53 AM, "Simon MacDonald" <si...@gmail.com> wrote:

>For those of us on Android using Eclipse we could standardize on:
>
>https://github.com/android/platform_development/tree/master/ide/eclipse
>
>Simon Mac Donald
>http://hi.im/simonmacdonald
>
>
>On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve
><ag...@chromium.org>wrote:
>
>> From the threads in the past on this subject, it seems like:
>>
>> -Style is an opinion, but:
>> -A consistent style would be nice even if it's not everyone's preference
>> -We don't want to shy away contributors by annoying them with style
>> concerns
>>
>> I think we could get some value out of using a command-line code
>>prettifier
>> and shared git hooks. The flow I'm envisioning here is:
>>
>> $ git commit ...
>> // git hooks complain about your style and tell you to run fix_style
>> $ bin/fix_style
>> $ git diff // Shows you what the reformatter did to you.
>> $ git add ...
>> $ git commit // passes the style check.
>>
>>
>> So:
>> 1. We should never have to complain about style when looking at pull
>> requests
>> 2. We should get a consistent style
>> 3. In the least, I'd like to see consistent tabs v spaces & no trailing
>>ws.
>>
>> One thing we may have to do is ask a contributor to enable the git
>>hooks.
>> AFAIK there is no way to enable shared git hooks for a project by
>>default,
>> so what we'd have is a directory called "hooks" and you'd need to
>>create a
>> symlink for it into your .git/ directory.
>>
>> If no one objects, then I'll pursue this today/tomorrow. My plan is to
>> start with whitespace (sed or awk), get that working, and then see what
>> x-platform open-source formatters exist out there.
>>
>> Andrew
>>


Re: Code Prettifiers

Posted by Simon MacDonald <si...@gmail.com>.
For those of us on Android using Eclipse we could standardize on:

https://github.com/android/platform_development/tree/master/ide/eclipse

Simon Mac Donald
http://hi.im/simonmacdonald


On Mon, Aug 27, 2012 at 10:33 AM, Andrew Grieve <ag...@chromium.org>wrote:

> From the threads in the past on this subject, it seems like:
>
> -Style is an opinion, but:
> -A consistent style would be nice even if it's not everyone's preference
> -We don't want to shy away contributors by annoying them with style
> concerns
>
> I think we could get some value out of using a command-line code prettifier
> and shared git hooks. The flow I'm envisioning here is:
>
> $ git commit ...
> // git hooks complain about your style and tell you to run fix_style
> $ bin/fix_style
> $ git diff // Shows you what the reformatter did to you.
> $ git add ...
> $ git commit // passes the style check.
>
>
> So:
> 1. We should never have to complain about style when looking at pull
> requests
> 2. We should get a consistent style
> 3. In the least, I'd like to see consistent tabs v spaces & no trailing ws.
>
> One thing we may have to do is ask a contributor to enable the git hooks.
> AFAIK there is no way to enable shared git hooks for a project by default,
> so what we'd have is a directory called "hooks" and you'd need to create a
> symlink for it into your .git/ directory.
>
> If no one objects, then I'll pursue this today/tomorrow. My plan is to
> start with whitespace (sed or awk), get that working, and then see what
> x-platform open-source formatters exist out there.
>
> Andrew
>