...
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.
(See the end for the ** footnote.)
Review window
- Avoid same day approval unless change is fixing an broken job or critical bug bug.
- As committers are spread across different timezone, provide atleast at least a 24 hours hour window before merge for merging any non-critical updates.
- Include other committers (if not already added into review) and primary contributors for reviews reviews.
Configure Gerrit watch for auto-notification
- Goto Go to 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.
- Also check the Gerrit dashboard for yourself.
Check the Commit Message:
- is Is the JIRA ticket issue related to this fix?
- does Do either the issue or the commitment message properly describe what is being changed and why?
- For self-release yaml, ensure comment include includes a description of changes/bug fixes introduced in that version. If multiple jira's are consolidated - include all Jira reference/all Jira references and a brief summary.
Check the License Blocks and Copyright Lines on all code and documentation files
- is Is there a LICENSE.txt file?
- All code modules should have comments at the beginning that look like this, using the appropriate comment convention: **
|
...
|
- Does it Do they mention the current year for the company doing the modification?
- Note that there is no separator between Copyright lines by different companies.
- Note that the word "Copyright" is capitalized.
- 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"
- Do ALL of the copyright notices for the current gerrit push document the same company? **
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 is preferred on all public methods, classes and modules (files)
...
- The repos need to have the version number expressed in multiple places
- In pom.xml, the project/version value is always specified as either W.X.Y or W.X.Y-SNAPSHOT, as in 1.3.2-SNAPSHOT. **
- Every directory that has a pom.xml file should ALSO have a version.properties file AND a ChangeLog.md file. **
- The exception is when there are subdirectories with individual pom.xml files. In that case, those directories should have a ChangeLog.md file. Typically these match the docker containers.
- The same value (without any "-SNAPSHOT" suffix) will be specified in version.properties, separated out into separate major, minor and patch values: **
- major=W
- minor=X
- patch=Y
- The same value will be specified in the ChangeLog.md file. **
- These values MUST match. **
- When an artifact (e.g docker container) is "released", a corresponding file will be added to the releases directory. The version of that release file must also match the version found in the pom.xml, version.properties and ChangeLog.md files. **
Check the Build Console Output
...
- The name of the changelog may be specified in any case, but at least the letter "C" must be capitalized. We are using markdown, so the extension must always be "
.md". - Within the changelog:
- The date stamp must always be specified as either YYYY-MM-DD or YYYY/MM/DD. **
- The JIRA ticket associated with the changes that went into a version should be noted. **
- It is not necessary to have both the JIRA ticket number AND a link to the JIRA ticket.
- There should be an entry for every single version.
- The latest version must always be first, in reverse time order. **
- The date stamp should always reflect the most recent change associated with a given version.
- An [Unreleased] section is optional. (JIRA tickets should be used in lieu of an [Unreleased] section.)
...
- Notify other committers/PTL on vacation plans
- Set status accordingly in gerrit Gerrit (under Gerrit->Setting→ Profile)
- Update weekly meeting "vacation" section specifying the OOO days/weeks
** Footnote
The tool onap-gerrit-review can be found on Github. It automates many of the checks listed above, in particular those marked with "**". It can be used by committers on code that needs to be reviewed, as well as by code submitters before they submit the code.
If you use the tool and have suggestions for improvements, please provide issues or pull requests on Github.