Contribution guidelines

(WORK IN PROGRESS)

As an open source project, we appreciate pull requests from the community. We don't have resources to fix every bug and implement every imaginable feature ourselves. On the other hand, every bit of code has to be reviewed and later maintained. Pulling unfinished, buggy or unservicable code would inevitably compromise stability of the software and make the future development difficult. For this reason, we cannot accept any pull request that comes. If you're a SW developer and consider contributing, this is a guideline to follow to increase the chance of merging your work.

General guidelines for a good pull request

Click to see details:

Try not to introduce additional config parameters

It is relatively simple to solve a corner case by adding a parameter that the user can tweak it with. That's not the preffered way. PrusaSlicer would become too complex, the workarounds would conflict and people would not know how to use your improvement anyway. We will not merge it unless it is very well justified.

Don't do any architectural changes

You may see an opportunity to improve the architecture by rewriting it. You may even be right and competent enough to do it. However, such a pull request would be huge, virtually impossible to review or test completely and quite likely to break something. We will not risk merging it and then suffer the fallout.

Try to do the changes as locally as possible

We are hesitant to merge PRs that change 20+ files of well-tested code for the same reasons as mentioned above.

Keep the number of changes to a minimum

Have you found a bug? Fix it. Nothing else. Don't do anything not directly related to that. Do not fix comment-typos in all files that you see, don't unify whitespace usage, don't move several unrelated classes to a different file because they fit there better and don't refactor everything that you stumble accross, no matter how obvious you think it is. A large pull request takes longer to review, is difficult to review (reason of such changes is not obvious) and there is a risk of breaking something. File separate (and orthogonal !) pull requests if you feel there is more to improve. Consider squashing your branch before filing a PR.

Document your changes

Commit messages and code comments should make the reasons for your changes clear. Are you fixing a bug reported on github? Reference it in the commit message. Do you know when it was introduced? Reference the first bad commit. Have you added a non-trivial block of code to recognize and solve some corner case? Mention it in the comment. And when you post your PR on github, clearly describe what you were trying to achieve and why.

Test your changes

We have seen pull requests that did not do what they claimed at all. We have seen pull requests failing tests that they themselves provided. We don't like them. Properly test your improvement and anything you think could have been broken (no matter how unlikely you think it is). There is currently no CI build system for pull requests, so don't expect to iterate to the correct solution by trial and error by misusing it. If your code fails to compile on OSX because of a missing #include that you didn't need on Windows, that's fine. A missing semicolon indicates that nobody ever compiled and tested the code. That's not fine.

Coding style

PrusaSlicer is a project that has its history. The coding style is not at all consistent among the source files and we are not especially worried about it, as long as the code works and is readable. Anyway, adhering to following advice in new code (!) would be appreciated:

• use 4 spaces for indentation, no tabs
• don't use braces to enclose just one statement
• opening brace is on the same line as previous if/while/... statement. Only function definitions use an extra newline.

void ExampleClass::how_to_do_it() const
{ // on a separate line
    if (m_something)
        std::cout << "no braces" << std::endl;
    else {
        std::cout << "now it is needed... ";
        std::cout << "but not on a separate line" << std::endl;
    }
}

• keep lines reasonably short - if you exceeded 80 chars, it is usually time to start thinking about a line break (this is a stupid advice in case the code is heavily indented - use your own judgement)
• naming and case: use MyType for types, my_variable for variables
• class member variable names are prepended by m_, except for simple POD structs

General C++ guidelines

Consider going through C++ Core Guidelines by B. Stroustrup & H. Sutter. We are all learning. To highlight what we consider important for pull requests revisions:

• Use const. A method that can be const should be. A reference or pointer that can be const absolutely should be. It is not needed to make every possible local variable const, although it might improve readability in some cases (and make it worse in others). Try to keep use of const_cast and mutable to a minimum.
• Prefer const member functions over non-const member functions and static non-member functions over them.
• Use RAII
• Use modern C++17, but not at all costs
• Only use auto where it helps:

    auto some_var = function_i_never_heard_of();   // BAD  (type not immediately obvious)
    double some_var = function_i_never_heard_of(); // GOOD (the two extra letters won't hurt)
    
    std::vector<const ExtrusionEntityCollection*>::const_iterator eec_it = ee_collections.begin(); // BAD  (way too verbose)
    auto eec_it = ee_collections.cbegin();         // GOOD (the type is clear enough)

• Don't homebrew what is already implemented in PrusaSlicer or some library it depends on (STL, Eigen, Clipper, boost, etc.)