Automated code review

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?

• Automated coding style checks using local_codechecker
• Automated phpdocs checks using local_moodlecheck

Will an issue be automatically rejected if cibot checks fail?

No. Developers should strive to have cibot approve every patch submitted but on some occasions cibot will detect issues present in existing code which are not expected to be fixed (see next item).

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 periodically 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.

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)

See also

• Coding style and other links in the coding guidelines category