After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 761465 - Start a discussion to ensure/fix coding style
Start a discussion to ensure/fix coding style
Status: RESOLVED FIXED
Product: seed
Classification: Bindings
Component: libseed
git master
Other All
: Normal minor
: ---
Assigned To: seed-maint
seed-maint
Depends on:
Blocks:
 
 
Reported: 2016-02-02 16:54 UTC by Danilo Cesar
Modified: 2016-02-11 02:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
huge formatting patch and adding clang-format tool (1.65 MB, patch)
2016-02-03 14:26 UTC, Danilo Cesar
none Details | Review
huge commit with formater setup... (1.64 MB, patch)
2016-02-03 19:14 UTC, Danilo Cesar
none Details | Review

Description Danilo Cesar 2016-02-02 16:54:54 UTC
So, at the current moment, the coding style on seed is bad, really bad.

it uses all the kinds of things, tabs for indentation, 4 space for indentation, 2 spaces for identation. GNU brackets, normal indented brackets. Comments with mixed space and tabs.

All the kind of things, and TBH, I'm also the author of some of those mistakes too.

I would like to make a suggestion:

Include a doc/CodingStyle.txt documenting the kind of style we want and adding a .clang-format to ensure that every piece of code follows that format.

As an example, GStreamer already uses gnu-indent as a git commit hook to ensure that each commits passes gnu-indent rules before entering the git tree. We could do something similar to that, but using clang-format, which is way more powerful/flexible.
Comment 1 Alan Knowles 2016-02-03 01:44:56 UTC
Yes, It's pretty messy.

I think the original code used tab+2 spaces (it's a wierd combo that gnu uses I think).

Trouble is that only I believe emac's and vim seem to support this format. almost any other text editor has serious problems with it.

Do you have any good suggestions for a standard coding style (that's easy to use...)

may as well keep discussion here.. My timezone is a bit odd (GMT+8) - so chat's always a bit hit/miss..
Comment 2 Danilo Cesar 2016-02-03 11:29:32 UTC
To be very honest, I don't care *too much* about coding style as long as we keep using the same one and I have a way to fix all my code and avoid spending time with coding style fixes.

A while back when I was contributing to Firefox I remember that I could use whatever I wanted and at the end I invoke the clang-format tool I mentioned to format the patch.

Lots of other big opensource projects use auto format tools to enforce coding style. Some have their own tool (like linux kernel and webkit), which would be overkill for us. Some others uses clang-format (like firefox).

We could spend some time discussing which coding style is better and do not arrive to any conclusion or definition. Or, we could get one that it's already there. 
My suggestion is to get the one from Mozilla (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style), add the .clang-format with the mozilla format (basically https://github.com/mozilla/gecko-dev/blob/master/.clang-format).

That one uses spaces to indent. I've have been looking around and feels like indent-tabs are not being used by many big project anymore (despite linux kernel).

I've added Richard to the discussion as well, as he's mentioned as one of the maintainers. However, I'm not sure if he's reading the emails...

So, Alan, if you agree with that I can prepare a coding style document for us and a .clanf-format to auto format the code.

Then, there's the big decision: Should we apply the format to all files right now? There's a good part of me that would love to do that. However, we will have to live with the fact that git blame won't be super useful anymore.
Comment 3 Danilo Cesar 2016-02-03 14:26:57 UTC
Created attachment 320341 [details] [review]
huge formatting patch and adding clang-format tool
Comment 4 Danilo Cesar 2016-02-03 14:30:14 UTC
Most of the changes are indentation only, so git blame -w is still useful...
Comment 5 Danilo Cesar 2016-02-03 19:14:34 UTC
Created attachment 320388 [details] [review]
huge commit with formater setup...
Comment 6 Alan Knowles 2016-02-04 02:04:13 UTC
I'm fine with this, Sent an email to the seed mailing list... - If no-one else objects... Let's go with it on monday.
Comment 7 Danilo Cesar 2016-02-09 13:00:07 UTC
Could you apply this Alan?

Thanks,
Comment 8 Alan Knowles 2016-02-11 02:05:38 UTC
commited - we just had Chinese new year here... everything closed for a few days.

Regards
Alan