learningequality / kolibri-installer-windows

Repository for Kolibri windows installer
MIT License
5 stars 12 forks source link

Coding style inconsistencies #136

Open cpauya opened 5 years ago

cpauya commented 5 years ago

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.

benjaoming commented 5 years ago

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 commented 5 years ago

+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 commented 5 years ago

@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 commented 5 years ago

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

language: python
python:
  - "3.7"
install:
  - pip install cpplint
script:
  - cpplint windows/
cpauya commented 5 years ago

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

indirectlylit commented 5 years ago

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