Pull Request Guidelines

Status

DONE

Stakeholders

All developers

OutcomeAgreed
Created date

  

Owner

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/ )