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. 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)
7. Performance - we have to look at maintaining optimum code here, as far as simple patches that can affect performance. (simple optimisations)
8. Scalability - if master only - we're looking far future , stable branches may not be lucky to get such improvements..
9. git authorship is correct vs committer + credits due are mentioned + email addresses
10. Documentation / phpdoc / readability
11. Tracker_issue_labels which might need adding. Particularly:
    1. docs_required
    2. ui_change
    3. api_change
    4. qa_test_required
12. 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)
13. Update the workflow counters
    1. If an issue is in need of a second review +1 to errors column in integration workflow counters