Style guide

13 views
Skip to first unread message

Ignasi

unread,
Oct 26, 2012, 12:16:52 PM10/26/12
to jclou...@googlegroups.com
Hi!

Recently there have been several pull requests with commits that had formatting issues.
I've opened a pull request [1] with an initial version of a style guide for the code, in order to have a public documentation with the preferred formatting rules.

Currently it only contains the two formatting rules that are used (AFAIK): use 3 spaces indentation and a 120 characters line wrap.
If there is something missing, let me know and I'll update the doc.


Thanks!

Ignasi


Andrew Gaul

unread,
Oct 26, 2012, 1:20:47 PM10/26/12
to jclou...@googlegroups.com
CloudBees can enforces these rules via mvn checkstyle:check.  Unfortunately jclouds has 1,895 violations for line length alone.  I sent a pull request which demonstrates how to enforce rules although we need to decide whether we want to go down this path.  Some of the checks line line length, indentation, and variable naming are common-sense but others degenerate into bike shed discussions.

Matt Stephenson

unread,
Oct 26, 2012, 3:21:05 PM10/26/12
to jclou...@googlegroups.com

You can make it so the verify phase does this.  Docs don't help, and we already have a page for eclipse development.  If we want to help with code style, setup check style and add it to the verify phase.  I can work on this over the weekend if people want help.

Docs don't help for code style, fail the build.

--
You received this message because you are subscribed to the Google Groups "jclouds-dev" group.
To view this discussion on the web visit https://groups.google.com/d/msg/jclouds-dev/-/ttdk8sB5Vj0J.
To post to this group, send email to jclou...@googlegroups.com.
To unsubscribe from this group, send email to jclouds-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/jclouds-dev?hl=en.

Matt Stephenson

unread,
Oct 26, 2012, 5:54:11 PM10/26/12
to jclou...@googlegroups.com

Fyi: me and Gaul discussed in IRC and it sounds like check style is the best route.  It provides support for many IDEs including eclipse and allows us to move in the direction of enforcing this at build instead of burdening the reviewer.
I can look at reformatting changes, any help is appreciated.

Ignasi

unread,
Oct 26, 2012, 6:09:50 PM10/26/12
to jclou...@googlegroups.com

Those are complementary things.
Making the build fail is more radical, and I don't know if it would introduce too much innecessary noise to pull requests (don't know if there is a need to be so strict on formatting right now). Anyway, I don't have a strong opinion on this.

Having the stuff documented in the site, however, is always good (there is no reason for not documenting). I think having a reference page will help us centralising the info and people setting up the environment before breaking the build.

Ignasi

unread,
Oct 26, 2012, 6:11:45 PM10/26/12
to jclou...@googlegroups.com

Good! I can help in reformatting too.

Andrew Phillips

unread,
Oct 26, 2012, 11:00:36 PM10/26/12
to jclou...@googlegroups.com
> Fyi: me and Gaul discussed in IRC and it sounds like check style is the
> best route. It provides support for many IDEs including eclipse and allows
> us to move in the direction of enforcing this at build instead of burdening
> the reviewer.
> I can look at reformatting changes, any help is appreciated.

This could optionally also be something we add to a profile that is
activated in BuildHive and perhaps an additional style build. This
would allow us to check especially pull requests easily.

ap

Matt Stephenson

unread,
Oct 26, 2012, 11:10:23 PM10/26/12
to jclou...@googlegroups.com
Ignasi,
I would rather commit to us upholding a strong style in jclouds, or not worry about this at all.  If we're going to commit to it, I'm willing to put in the time this weekend to make that happen.  If we're not going to commit to it, I wouldn't bother with a solution that only helps a subset of our community if they bother to read the docs site.  Working internally with a project is much different than working on an opensource project.  Our regular contributors mostly already know the style guide and don't need a reference on the docs site.  Drive-by-contributors won't bother reading it, and the end result will be us referencing it constantly in code reviews.

I think that if we were an internal organization at a company doing this, it would work just fine, but since this is a public facing project, the dynamics are quite different.  I'd rather just own-up and commit to building in some strong check style rules.  I'm willing to put in the time to make this happen.

Thanks,

Matt Stephenson
jmstep...@gmail.com

Matt Stephenson

unread,
Oct 26, 2012, 11:14:13 PM10/26/12
to jclou...@googlegroups.com
Andrew,
Totally, this is exactly how we kept things in-step at Amazon while I worked there.  We used hudson to run the checkstyle rules and emailed developers if they had style issues.

Thanks,

Matt Stephenson
jmstep...@gmail.com


--
You received this message because you are subscribed to the Google Groups "jclouds-dev" group.
To post to this group, send email to jclou...@googlegroups.com.
To unsubscribe from this group, send email to jclouds-dev+unsubscribe@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages