-
Notifications
You must be signed in to change notification settings - Fork 389
Code Review
Michael J. Witte edited this page Nov 23, 2015
·
4 revisions
Developers are not supposed to merge their own pull requests. In general, a separate developer will review the changes and do the actual merge. The following checklist is guide for what a reviewer should do once a pull request is ready to be merged.
- Review the CI test reports
- Run the defect files first with the latest release (or current develop) to confirm the original failure.
- Pull down the code and do a debug build and run the defect files to make sure everything runs ok.
- Build energyplus_tests and run it as a full set (no filter).
- Review the code changes and unit test and comment as needed.
- Merge
- Delete the branch
- Make sure the issue(s) addressed is closed (might close automatically if referenced in a commit). If not closed, then close with a comment like "Closed via PRnnnn"
- Accept on Pivotal
- Apply one of these Labels to the issue(s) addresed (used to sort the changelog or exclude a PR that we don't need to publish): Defect, NewFeature, Performance, Refactoring, DoNotPublish.
- Edit the pull request and any issues it addressed so that the title and first comment field describe the change. These are used to automatically generate documentation of what has changed and should be meaningful to users and reflect what the actual problem was or what was changed.