In addition to the guidelines specified Committer Best Practices, all DCAE committers are expected to look at these areas before merging a patch. If any item is not followed, you should reply with a -1. Also add comments in appropriate places within the code.
Review window
- Avoid same day approval unless change is fixing an broken job or critical bug
- As committers are spread across different timezone, provide atleast 24 hours window before merge for non-critical updates.
- Include other committers (if not already added into review) and primary contributors for reviews
Configure Gerrit watch for auto-notification
- Goto Gerrit→ Settings→Notification to setup auto email alerts to recieve notification on submission into DCAE repository. This will help when submitter miss to add explicitly all the committers for reviews
Check the Commit Message:
- is the JIRA ticket issue related to this fix?
- does either the issue or the commitment message properly describe what is being changed and why?
- For self-release yaml, ensure comment include description of changes/bug fixes introduced in that version. If multiple jira's are consolidated - include all Jira reference/brief summary
Check the License Blocks and Copyright Lines on all code and documentation files
- is there a LICENSE.txt file?
- All code modules should have comments at the beginning that look like:
# ================================================================================ |
- All documentation files should have comments at the beginning that look like:
.. This work is licensed under a Creative Commons Attribution 4.0 International License. |
- Does it mention the current year for the company doing the modification?
- Note that there is no separator between Copyright lines by different companies.
- Note that when a company updates code that was previously copyrighted by them, the date range should be extended as shown.
- There is no alternate wording for the copyright lines, such as "modifications copyright"
Check for comments in all new code
We should improve the documentation in the code whenever possible. At a minimum, NEW code should be documented.
- javadocs, pydocs format preferred on all public methods and classes
Check the code
- Most importantly, does it actually fix what the commitment message says should be fixed?
- Verify against ONAP code standards.
- Java style is specified here, which points to the Google Style Guide repo.
- Javascript code should meet the similar Javascript Google Style Guide, except for the number of characters in one line of code is not restricted.
- Python code should similarly match the Python Google Style Guide.
Check whether the version number should be updated
- if there is any new feature/enhancement
- the patch version should be bumped
- if a change is a bugfix on a previously merged patch, AND if that previous version is not already released
- then version change is optional
- different repos need to have the version number expressed in multiple places
- document those . . . TBD
Were there any -1 on previous patches by another committer or the PTL?
- This is a FULL STOP.
- Please DO NOT merge the code until:
- that other committer has given at least a subsequent +1 or
- the PTL says it is okay to +2 anyway (which would be very rare)
Self-Release yaml
- Ensure no outstanding patch remaining in gerrit for review/merge for container being released. We would want to avoid releasing container too frequently as well (consider consolidating release for multiple patch)
- As artifiact release impacts different repositories (blueprint/bootstrap, oom etc); consolidate release request (and subsequent update to other impacted repositories)
- For self-release yaml, ensure comment include description of changes/bug fixes introduced in that version. If multiple jira's are consolidated - include all Jira reference/brief summary