Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coding style inconsistencies #136

Open
cpauya opened this issue Aug 16, 2019 · 6 comments
Open

Coding style inconsistencies #136

cpauya opened this issue Aug 16, 2019 · 6 comments
Labels
TODO: needs decisions Design or specifications are necessary TODO: needs user docs Requires user docs update

Comments

@cpauya
Copy link
Contributor

cpauya commented Aug 16, 2019

It's always good to keep our coding style consistent.

Sources affected

Some issues with current code base

It makes code reviews a bit hard due to coding style inconsistencies. Here are some examples

  1. Code blocks use both inline or slanted versions. Let's use the "slanted version" for code blocks (maybe except class and method definitions). Here are examples:
    // if statement
    if ( <condition> ) {
        // code
    } 
    else {
        // code
    }

    // while statement
    while ( <condition> ) {
        // code
    }

    // class 
    class SomeClass : public BaseClass
    {
        // class code
    }

    // method
    void someMethod() 
    {
        // method code
    }

Notice the position of the opening curly brace { in the above sample codes?

  1. Pointer and Reference operators - in the code base, some declarations use char * script_name while some use wchar_t* wc = new wchar_t[cSize]; - let's use the latter format? So instead of char * script_name -- do this instead char* script_name. See reference: https://www.w3schools.com/cpp/cpp_pointers.asp

  2. Spaces instead of Tabs - it's a royal rumble between these two, let's use 4-spaces for code indents.

References

This is already a few years old but most of its recommendations are still very applicable to achieve C++ coding style consistency:

Final Notes

I recommend we use the reference from GeoSoft - C++ Programming Style Guidelines for a quick start - so we can "clean-up" this code base. Later, we can come up with our own C++ Coding Style Guidelines if needed and have it documented somewhere and mentioned in the README.

@cpauya cpauya added TODO: needs decisions Design or specifications are necessary TODO: needs user docs Requires user docs update labels Aug 16, 2019
@benjaoming
Copy link
Contributor

Would perhaps be most sustainable to find a simple C++ linting tool that can be automated with pre-commit hooks?

Using https://pre-commit.com, you could create a hook. The project expects a reference to a git repository, which I assume can be the current local one (path: .).

You could then write a script that invokes cpplint for all files contained in the changeset within the set of known C++ extensions.

Inspired by this script, calling cpplint: https://gist.github.com/brickgao/fb359764d46f9c96dd3af885e94b0bab

cpplint is a Python project: https://pypi.org/project/cpplint/

@cpauya
Copy link
Contributor Author

cpauya commented Aug 23, 2019

+1 Good one @benjaoming - For a "quick" solution, I also found out that VS Code has the Format Document feature (I just started using VS Code last week). This should also help achieve code consistency.

Screenshot 2019-08-20 20 43 28

@benjaoming
Copy link
Contributor

@cpauya I don't think that local code formatters are a good idea, although this is a project with a small circle of contributors.

If you want consistency, you would have to script it and enforce it somehow. Git pre-commit hooks can be a good start, you could also add Travis CI.

@benjaoming
Copy link
Contributor

Actually, you could also write a .travis.yml that simply said:

language: python
python:
  - "3.7"
install:
  - pip install cpplint
script:
  - cpplint windows/

@cpauya
Copy link
Contributor Author

cpauya commented Aug 23, 2019

Ah good point @benjaoming - since other (possible) contributor/s may not follow the same C++ format. 😺

@indirectlylit
Copy link
Contributor

Let's stick to quick solutions!

There's a fair chance we'll want to deprecate the C++ GUI code in favor of something more like what we're doing with the mac app for the next major upgrade:

https://github.com/learningequality/kolibri-installer-mac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs decisions Design or specifications are necessary TODO: needs user docs Requires user docs update
Projects
None yet
Development

No branches or pull requests

3 participants