As discussed during our appsec scrum today, we should consider adding clarifications to this SOP:
- A new section within Purpose describing code states that we will not review including:
- Any variety of stub code - be it a mediawiki extension, service, etc. Boilerplate code is just that and cannot serve as a proxy for reviewing code that may one day exist.
- Any Work-In-Progress (WIP) patch sets, regardless of their state of completion. If the code is in a stable, testable state, it should be code-reviewed and merged.
- Any relevant patch sets which have not been code-reviewed and merged. Again, if the code is in a stable, testable state, it should be code-reviewed and merged. We may be willing to make exceptions to this policy if a code merge is blocked on another, key review (say from a member of the Performance Team or SRE) but it will be the responsibility of the requester to ask for such an exception from the Security Team and confirm the current state of the code.
- A new section within the Submission and Timelines subsection of the Process section:
- Add another IMPORTANT note under the first item stating that a commit hash or version tag MUST be specified for every repository branch.. If the code links are for patch sets on gerrit et al, then see guidelines above. I'm pretty sure this is a simplified statement of the solution that was eventually arrived at in the "Add a 'code freeze' or ready for deployment clause" topic on this page.