iamgreaser / iceball

Open-source rewrite of the VOXLAP version of Ace of Spades.
http://iceball.build
GNU General Public License v3.0
113 stars 32 forks source link

Write down coding conventions #230

Open fkaa opened 8 years ago

fkaa commented 8 years ago

Currently I see mixed tabs and spaces, non-consistent function names and using POSIX reserved struct names (eg. foo_t). It would be nice if we could formalize it and write down some guidelines in the wiki.

Some things that do seem to be consistent:

Some things I would like to see:

NotAFile commented 8 years ago

https://github.com/iamgreaser/iceball/blob/master/docs/dev_styleguide.txt

could be copied to the wiki

fkaa commented 8 years ago

Nice catch, didn't notice that one. I'd still argue for spaces though ;)

NotAFile commented 8 years ago

unfortunately for you greaser is a) bdfl b) the guy who wrote most of the code c) probably not willing to change his coding conventions because one guy dislikes them

rakiru commented 8 years ago

c) probably not willing to change his coding conventions because one guy dislikes them

Make that two people. ;)

NotAFile commented 8 years ago

make that three people

fkaa commented 8 years ago

I think we can take a lot from the Stronglink C Guidelines (very extensive).

asiekierka commented 8 years ago

Follows a simple tutorial of moving the codebase to 4 spaces:

  1. Open your editor settings.
  2. Set the tab width to 4.

This tutorial can also easily be adapted for getting a codebase of 2, 6, 8 or 9 spaces.

rakiru commented 8 years ago

@asiekierka The hard part is convincing grease to change anything. I know few people more stubborn than I, but I'm pretty certain he's one of them.

As much as I'd love to use tabs, they simply aren't as practical as spaces[1]. You can't align things with tabs, such as continued lines. Github also uses the (historically correct) standard of 8 columns, which makes reading code on here a pain in the dick.

The current style guide is also a bit weird. I mean, what is the point of adding this edge-case?

Braces start on a new line, except for...

Either way, consistency across the code base is the most important thing, so having a better style guide would be good. Especially for Lua, since it's currently just "do whatever the fuck you want, except use Lua's builtin "class" system".

[1] I say this as someone who has used tabs for the majority of their programming life.

iamgreaser commented 8 years ago

We are not fucking using 4-space indentation. It's 8-tab hard tabs of whatever width you like, just don't assume a given width. This will not change.

As for warnings, I'm not a fan of -Werror as it's a surefire way to watch the code become uncompilable with a compiler 5 years down the track, but -Wall -Wextra's good, and telling devs to make sure their code compiles without warnings is also good.

Which leaves these:

Prefix externally visible functions with appropriate namespace (eg. ib_foo())

I have no issue with this.

I'm just mostly preoccupied with other projects right now, so I'll probably not be doing this in a hurry, but you've got the green light.

Either way, consistency across the code base is the most important thing, so having a better style guide would be good. Especially for Lua, since it's currently just "do whatever the fuck you want, except use Lua's builtin "class" system".

Feel free to brainstorm ideas, because it could do with some consistency.

Oh and this:

The current style guide is also a bit weird. I mean, what is the point of adding this edge-case?

To me it looks better. Yeah, not really a very logical reason for it. If you want to change it, bring forth a few examples of different brace styles and we could possibly use the one that's least shit.

rakiru commented 8 years ago

A) Current (Feel free to edit if it's wrong)

    void derp(int herp)
    {
        if (herp == 0)
            something();
        else if (herp == 1) {
            something();
            something_else();
        } else
            something();
    }

B) My preferred

    void derp(int herp) {
        if (herp == 0) {
            something();
        } else if (herp == 1) {
            something();
            something_else();
        } else {
            something();
        }
    }

C)

    void derp(int herp)
    {
        if (herp == 0)
        {
            something();
        }
        else if (herp == 1)
        {
            something();
            something_else();
        }
        else
        {
            something();
        }
    }

D)

    void derp(int herp)
    {
        if (herp == 0)
            something();
        else if (herp == 1)
        {
            something();
            something_else();
        }
        else
            something();
    }

etc.

The problem I have with optional braces is that it's really easy to add a second line in there and forget to add the missing braces.

I prefer opening braces on same line as it wastes less vertical space. The argument for new line braces is usually so the opening and closing ones line up, but my counterpoint to that is that it lines up with the line the block starts on anyway.

As for Lua, does Lua have an "official" style guide of any sort? It may be a good idea to follow that, since most existing Lua suers are probably familiar with it, but I'm not sure what it's like, if it exists. I'll look into that.

fkaa commented 8 years ago

@rakiru I've taken a liking to D with mandatory braces (1TBS) after going through K&R, but it feels a bit redundant (imo) since we can't align parameters on newlines due to hard tabs (unspecified width).

That said, I wont have a problem with whatever gets chosen, just thought it would be good to nail the specifics down.

asiekierka commented 8 years ago

@rakiru I am most fond of B myself.

2016-01-07 1:15 GMT+01:00 Felix Kaaman notifications@github.com:

@rakiru https://github.com/rakiru I've taken a liking to D with mandatory braces (1TBS) after going through K&R, but it feels a bit redundant (imo) since we can't align parameters on newlines due to hard tabs (unspecified width).

That said, I wont have a problem with whatever gets chosen, just thought it would be good to nail the specifics down.

— Reply to this email directly or view it on GitHub https://github.com/iamgreaser/iceball/issues/230#issuecomment-169505825.