DCAE Committer Guidelines

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:

• 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 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"

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.
• Did the unit tests get updated?
  • New functionality MUST have unit tests.
  • New methods added to existing code MUST be covered by existing or additional unit tests.
  • We are looking for a minimum of 60% code coverage, with higher levels encouraged.

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 changing the version is optional
• 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.
  • 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.

Check the Build Console Output

• Errors during the build process will cause the verify step to post a -1, preventing merger.
• However, there are a other things to still look for in the build console output.
  • In java and python code, does the "lint" pass look okay?
  • Look for "hidden pieces of coal". Lots of stylistic issues (say about indentation and white space) can hide real issues.
• Are the unit tests running cleanly?
  • For example, you shouldn't see stack traces from unit tests, as the unit tests should capture those.
• Check the code coverage statistics when available.
  • Python code will generally show code coverage statistics in the build console output.
  • Make certain that we don't fall below 60% code coverage. (Numbers closer to 80% are encouraged.)
  • Java code usually does not show in out build logs, but are currently generated after merger.

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

ChangeLog.md file

[This section is not yet agreed to.]

In the Istanbul release, we have adopted the practices of keeping a changelog. Best practices for changelogs can be found at <keepachangelog.com>.

An example of a Changelog is:

As committers, some of the things you should always check are:

• 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.
  • There should be an entry for every single version.
  • The latest version must always be first, in reverse time order.
  • An [Unreleased] section is optional. (JIRA tickets should be used in lieu of an [Unreleased] section.)

Vacation Notice

• Notify other committers/PTL on vacation plans
• Set status accordingly in gerrit (under Gerrit->Setting→ Profile)
• Update weekly meeting "vacation" section specifying the OOO days/weeks