Openspades: Style guide

Created on 19 Sep 2013  路  14Comments  路  Source: yvt/openspades

I didn't find any style guide in the project, would be a good idea to have one at some point.

discussion refactoring

Most helpful comment

Oh yes, the code base is really inconsistent right now.

We should make a style guide already. OpenSpades has grown really big and, in my opinion, it's hard and annoying to deal with code with different consistencies.

What annoys me the most is that some parts have spaces before braces and others don't, sometimes in the same code.

All 14 comments

ideally adding a project file for this: http://astyle.sourceforge.net/
so you can run code trough that before commit / pull request.

relatedly, http://en.wikipedia.org/wiki/Travis_CI This could be usefull and integrates REALLY well with github

We have Travis now so I'm closing this.

@VelocityRa Actually the problem is still there and Travis only ensures that openspades builds on Linux, which is unrelated to this issue.
Currently the code base is a mixture of space/tabs indentation. I propose to use clang-format or editorconfig(for some basic settings like tabs/spaces) or both(clang-format for C++ and editorconfig for angelscript).

Oh yes, the code base is really inconsistent right now.

We should make a style guide already. OpenSpades has grown really big and, in my opinion, it's hard and annoying to deal with code with different consistencies.

What annoys me the most is that some parts have spaces before braces and others don't, sometimes in the same code.

@VelocityRa I think this is a issue where the discussion tag would be appropriate.

Right, sorry for closing this then.
Definitely in favour of clang-format. Most files I tested become smaller and easier to go through when clang-formatted.
Main.cpp was an exception, it goes from 680 to 742 LoC.

@yvt thoughts on this?

Haven't tried clang-format and editorconfig yet, but they look good to me.

The LLVM coding standards look good to me.
For the other options available check this.
Maybe it would be better to use LLVM, but with IndentWidth set to 4 spaces?
Webkit looks fine as well.

I suggest using clang-format only on modified or new code, because otherwise it would break git blame and similar tools. clang-format includes a nice script which only formats changed lines.

@theunknownxy

1) I think it's fair to say that most of the code has already been written. A lot of commits from now on will modify and maintain that code.

2) About the git blame thing specifically: Most of the code is YVT's anyways. Would it really matter that much? Besides, you can see the history of each file anyway, git blame is just a small convenience.

Given those points, I disagree, and I also think that it would make sense for @yvt to make the formatting commit.

git blame is not (only 馃槈) about blaming a person, it's more about seeing why something was changed. But of course it's yvt's decision.

Closing this, as #471 fixes it. (400 issues/pull requests later! lol)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

parkerlreed picture parkerlreed  路  4Comments

CaptainToon picture CaptainToon  路  4Comments

CarbonCookies picture CarbonCookies  路  7Comments

Mailaender picture Mailaender  路  7Comments

NotAFile picture NotAFile  路  5Comments