Note: This page is a work-in-progress. Feedback and suggested improvements are welcome. Please join the discussion on moodle.org or use the page comments.

Moodle issues submitted for review and inclusion into core are examined by our continuous integration (ci) system and checked against various automated tests. We use the automated checks to help improve the code quality in Moodle.

What is CiBot?

CiBot is our automated code checker and it will run checks against any issues waiting for peer review, sent for integration or requested to be checked by the developer. It will report problems discovered in lines of your patch which you are changing.

What checks is CiBot carrying out on each issue?

Verify mergeability, number of commits and branch ages.
Require testing instructions.
Automated php syntax using php -l
Automated coding style checks using local_codechecker.
Automated PHPDoc checks using local_moodlecheck.
Automated commit message checks using these scripts.
Automated upgrade savepoint checks using these scripts.
Automated jshint javascript linting checks using the .jshint configuration shipped with Moodle
Automated third party library modification check using these scripts

Will an issue be automatically rejected if cibot checks fail?

No. Developers should strive to have cibot approve every patch submitted, but the integration team will take a pragmatic approach to things like:

Occasions where cibot will detect issues present in existing code (see next item)
Variations from commit message style which make the commit message more understandable overall

Should coding style issues in existing code be fixed?

In general, please only update lines relating to your own change. Policy for this situation is being discussed in MDL-43233.

Example situations

Spacing of if statements is incorrect on the line being changed

This can be corrected without affecting existing code, so should be fixed.

The line being changed contains an invalid variable name

If correcting this variable would affect other parts of the code not covered by the patch then its not reasonable to fix it.

Requesting CiBot checks an issue

At any time a developer can add the label 'cime' to an issue to request it runs it checks against it. The bot checks for issues with the cime label every 30mins and runs the checks then remove the cime label. (Note that because it removes the label, it is normal to 'create' the label).

Any issue submitted for peer review or integration review will be checked automatically as long as it does not already have the 'ci' label.

Are additional CiBot checks possible?

Yes. It is planned to add more checks - (see MDLSITE-3267) and developers are encouraged to suggest new checks in that issue. Note that some limitations exist:

The checks should be consistent and completely reproducible
The check should complete in minutes not hours (other solutions are in planning for longer term tests)

The git commit summary limit is too small

When you overrun the length limit many git tools do not display commits well. See how github truncates ea6f548081 - it ruins the message and makes it harder for you to communicate your change to other developers.

It is acknowledged that its often tricky to get a useful message in such a short space on the first line. However, the coding guidelines for git summary line length were established on the basis of industry best practice. As the Coding_style#Git_commits mentions, do not be afraid to go into much more detail in your commit body.

Try

git log --oneline --no-merges

if you want to see how other developers have tried to adapt to this situation.

Documentation

Automated code review

Contents