Pull Request Guidelines
NOTICE
This decision has been migrated to the Technical Council's Decision Log as part of a consolidation effort. See: DR-000017 - Pull Request Guidelines
This guide is intended to provide guidance of how pull requests for community maintained modules should be managed.
Minimum Expectations
- A pull request must be related to a JIRA issue in the appropriate project for that repository, unless the changes are minor e.g. correcting a typo, and does not effect production code execution in any way
- A pull request must include a reference to the JIRA issue in the initial title
These are both necessary in order to relate code changes to why the code needed to change (see below)
General Guidance
Relating code changes to why the change is needed
In order to understand what caused a code change to be made, it is important to be able to relate changes to the reason the code is being changed. This is especially useful especially when finding the code to back-port a bug fix or for investigating the cause of a bug.
It is also useful for setting some context for the changes for reviewers and for tracking which code changes are included in a module release (via the fix version in JIRA, see below).
- A pull request must be related to a JIRA issue in the appropriate project for that repository
- A pull request must include a reference JIRA issue in the initial title
- A branch name should include a reference to the JIRA issue
- Commits may include a reference to the JIRA issue
Sharing the intent of the changes
In order to help reviewers (and folks investigating code changes later on e.g. to understand the cause of a bug) understand both why the code needs to change and why / how it was changed in a particular way:
- A pull request should be submitted when the submitter considers it ready for review (e.g. tests have been written etc)
- A pull request should have a single, clear purpose. This may mean that a single JIRA issue needs one pull request to fully resolve it
- A pull request should have a description briefly explaining why and how the code is changing
- A pull request description may explain the design or approach taken
- Commits messages should explain the intent of the changes in the commit (e.g. please do not use a message like {{working on x feature}} for every commit)
Release Management
In order to understand what changes are in a module version and reduce the effort required to release it:
- The fix version of the JIRA issue should be updated when a pull request is merged as it takes considerable effort to fill this in later when absent
- At the discretion of the lead maintainer, it may be required that a branch contain updates to the news / change log so that does not need to be done at release time
Merging the Pull Request
The owner of the pull may choose to merge, squash or rebase the pull request. The most appropriate option should be chosen to aid maintenance in the future (e.g. understanding the change, back porting the changes)
The lead maintainer of the repository may provide guidance limiting these options for a specific repository. Any restriction must be documented in the readme
The branch should be deleted after a pull request has been merged in order to limit the accumulation of old and redundant branches in a repository
Back end Specific Guidance
Interface Changes
In order to help communicate and coordinate the impact of interface changes:
- Any interface change should be reflect in the version in the RAML and the module descriptor
- A breaking compatibility interface change must be merged in coordination with pull requests for dependent modules in order to minimise disruption to the hosted reference environments
- A breaking compatibility interface change should include a major version change to the module implementation version
Dependencies
In order to maintain the stability of builds and allow for modules to be released.
- A branch should not use SNAPSHOT versions of maven dependencies as these can cause instability in builds due to changes in the dependency
References
The use of must, should, may etc is based upon RFC2119
More thoughts can be found for good commit messages at https://chris.beams.io/posts/git-commit/ (taken from the existing documentation at https://dev.folio.org/guidelines/contributing/ )