<div dir="ltr">Hi JP,<div><br></div><div>First of all, welcome to the RIOT community and thank you for your contribution!</div><div><br></div><div>There are two steps to our CI at the moment. The first one is Travis, which provides you with some preliminary static check regarding our coding conventions, documentation and also some static code analysis. When someone was able to review your PR they will task our build bot Murdock, the second step, to try to build your PR for most known build configurations.</div><div><br></div><div>For now, I suggest that you review the results of Travis to make sure the PR is ready for review. Here are the most important ones to fix right now  (links are from the latest build of your PR as of writing this PR, you can view the most current version by clicking on "Details" next to the Travis build result at the bottom of the PR):</div><div><br></div><div>* Commit messages (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L705-L709">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L705-L709</a>): we are loosely enforcing the 50/72 rule [1]. In our case this means: a commit message should at most contain 50 characters, in special cases it may be longer, but never longer than 72 characters. It is also preferable to prefix the commit message with the module you manipulate / add</div><div>* Whitespaces (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L711-L1008">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L711-L1008</a>): The only whitespace a line should and with is `\n`. If you are on Windows and *have your Git configured as such* it is also possible to have `\r\n`, Git will than automatically strip the `\r`. Also use 4 spaces for indendentation and don't mix spaces and tabs</div><div>* License (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1011-L1030">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1011-L1030</a>): The code you provided seems to be Apache 2 license. Please make sure it is compatible to our LGPLv2.1 and update our license pattern list [2] accordingly, if so (or ask for help in the PR).</div><div>* C++ compatible headers (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1033-L1049">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1033-L1049</a>): RIOT has C++ support, thus it's headers need to be C++-compatible. Have a look [3] [4] at existing headers how to achieve this.</div><div>* cppcheck's static code analysis (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1051-L1055">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1051-L1055</a>): Always useful to review what it says. If it's a false positive suppress the warning. Ask in the PR how to do that, if that's the case.</div><div>* The PR check (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1057-L1077">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1057-L1077</a>) you can ignore for now. After the review is done, the reviewer will ask you to squash your commits down to a sensible number.</div><div>* Header guards (<a href="https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1080-L1097">https://travis-ci.org/RIOT-OS/RIOT/builds/318473955#L1080-L1097</a>): Header guards should be the path of the header file, starting after the "include", but all upper-case. E.g. boards/hifive1/include/sifive/hifive1.h would have the header guard `SIFIVE_HIFIVE1_H`.</div><div><br></div><div>In general, have a look at our coding conventions [5]. Most of what I paraphrased here is described there in more detail.</div><div><br></div><div>Cheers,</div><div>Martine</div><div><br></div><div>[1] <a href="https://gist.github.com/vecano/8494051#committing">https://gist.github.com/vecano/8494051#committing</a></div><div>[2] <a href="https://github.com/RIOT-OS/RIOT/tree/master/dist/tools/licenses/patterns">https://github.com/RIOT-OS/RIOT/tree/master/dist/tools/licenses/patterns</a></div><div>[3] <a href="https://github.com/RIOT-OS/RIOT/blob/master/boards/samr21-xpro/include/periph_conf.h#L30-L32">https://github.com/RIOT-OS/RIOT/blob/master/boards/samr21-xpro/include/periph_conf.h#L30-L32</a></div><div>[4] <a href="https://github.com/RIOT-OS/RIOT/blob/master/boards/samr21-xpro/include/periph_conf.h#L281-L283">https://github.com/RIOT-OS/RIOT/blob/master/boards/samr21-xpro/include/periph_conf.h#L281-L283</a></div><div>[5] <a href="https://github.com/RIOT-OS/RIOT/wiki/Coding-conventions">https://github.com/RIOT-OS/RIOT/wiki/Coding-conventions</a></div></div><div class="gmail_extra"><br><div class="gmail_quote">2017-12-18 20:47 GMT+01:00 JP <span dir="ltr"><<a href="mailto:jpbonn-keyword-riotos.ae0e7f@corniceresearch.com" target="_blank">jpbonn-keyword-riotos.ae0e7f@corniceresearch.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I just created a pull request for merging support for the SiFive HiFive1 board.  I've never used the pull request mechanism before so hopefully nothing is too buggered.  What need to be done for the CI integration? Those checks are failing.<span class="HOEnZb"><font color="#888888"><br>
<br>
JP<br>
______________________________<wbr>_________________<br>
devel mailing list<br>
<a href="mailto:devel@riot-os.org" target="_blank">devel@riot-os.org</a><br>
<a href="https://lists.riot-os.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">https://lists.riot-os.org/mail<wbr>man/listinfo/devel</a><br>
</font></span></blockquote></div><br></div>