Integration Review

Purpose

The purpose of the integration review is to:

• Ensure consistent quality across the codebase
• Ensure that pedagogical aims of Moodle are at the forefront of any change
• Take into consideration the holistic view of moodle, looking at the impact beyond where the original developer was focused
• Provide guidance and feedback to developers, helping them learn and improve
• Consider other perspectives of other users perhaps not considered by original developers e.g.
  • Teachers
  • Students
  • Administrators
  • Third-party developers

Integration Review Process

1. Run automated pre-checks against the continuous integration server. (In future this can be automated and also moved into publicly accessible domain.)
2. Final code review, much like the peer review, except this is the final check. To include
   1. Takes place in-situ (integrated) to examine the impact against other integrated issues
   2. Checking against the coding guidelines - syntax/whitespace
   3. Moodleisms - using the built-in api functions where appropiate
   4. Cross-DB compatibility
   5. Security
3. Check purpose - the bug needs to fix the issue reported.
4. Ensure backwards compatibility is is maintained. As a starting point backwards compatibility must always be maintained. Where backwards compatibility is affected it should be:
   1. Well discussed with evidence of justification
   2. Documented and communicated to the community
5. Check the right people have been involved (e.g. component maintainers)
6. For fundamental changes, check that a thread has been started in an appropriate forum, and other Moodlers given enough time to comment.
7. Tests - must be written to guide tester to verify the fix is working.
   1. Unit test - very much preferred if applicable
   2. at end of wednesday, ensure testing is going to complete as expected. else take other actions (speak to test manager)
8. Performance - we have to look at maintaining optimum code here, as far as simple patches that can affect performance. (simple optimisations)
9. Scalability - if master only - we're looking far future , stable branches may not be lucky to get such improvements..
10. git authorship is correct vs committer + credits due are mentioned + email addresses
11. Documentation / phpdoc / readability
12. Tracker_issue_labels which might need adding. Particularly:
    1. docs_required
    2. ui_change
    3. api_change
    4. qa_test_required
13. Fixed Version after integration - is the versions that the issue is patched for. (A rule here : mention master if its the only branched fixed, if not don't mention master)
14. Update the workflow counters
    1. If an issue is in need of a second review +1 to errors column in integration workflow counters

Integration Principles

Integration (non-technical but philosophical) principles (4-5 words determining if something has to be integrated/backported or no):

1. safety: if something does not look safe, stable, it won't land. Be conservative.
2. security: all security issues, if not breaking principle (1) will be integrated/backported to all security-supported versions.
3. community: Anything not useful for the community (or against it) won't be integrated/backported. We can measure the community as 10%HQ, 10%Partners, 10%Core developers, 20%Admins, 20%Teachers, 30%Students - not exact science, just one approximation, you know). Question yourself how the change will affect those groups and ensure positives are bigger always (only affected groups count). All community issues, if not breaking principles (1) and (2) will be integrated.
4. typology: bug fixes will be always integrated/backported to all the supported braches if none of the principles (1), (2) and (3) are violated. Also, partially-unsupported branches can receive some if they are important enough. Improvements and new features go, exclusively, to master only, that's the main reason for short release periods. We *must* not make exceptions to this.
5. priority: issues will be "ordered down" by priority down (where priority is a mix of various factors, dynamic). And will be integrated in that order. If something has to be delayed, better if it is low priority. Once again, nothing here can break any of the previous principles.

If all the principles are fulfilled, the answer for "should I integrate this?" is, "yes, please!" (apart from technical findings, of course, that can lead to the issue being not integrated/reopened at last, these principles are 100% philosophical)

Schedule

In normal periods, the integrators adhere to the following schedule:

• Sunday 22:00 UTC: Issues waiting for integration brought into current integration.
• Monday: Integration
• Tuesday: Integration
• Wednesday: Testing. Integrators duties during this time are to monitor, facilitate and 'problem solve' the testing process.
• Thursday: Testing should be completed by 07:00 UTC, at which time remaining testing failures will be reverted and reopened. The release process follows.

Note: With issues which have had the commits reverted in integration.git, developers should rebase their branch on top of the the new moodle.git release. Minor fixes should be logically chunked in the commits if possible.

• Friday: Should be kept free from integration. Integration systems are maintained during this time.