Skip to content

Pull Request Checklist

gavin edited this page Jun 18, 2020 · 47 revisions

Before you send a pull request, make sure to read the Contribution Guide

Which Branch

As of November 2019. Here are the branches where you need to send pull requests:

Version           Branch             		    Type of PR
-------           ------             		    -----------
10                v10.x.x                       bugfixes only
11                version-11-hotfix             bugfixes only
12                version-12-hotfix             bugfixes only
13                develop                       bugfixes and new features

Pull Request Titles and Commit Messages

We follow Conventional commits and a status check evaluates your PR title and the commit messages. If commits don't follow the conventional commits specification, the PR, if it follows the same will be squashed.

Template for commit messages which can be extended for PR titles as well:

<type>[optional scope]: <description>

[optional body]

[optional footer(s)] 

Test Cases

Important to add test cases, even if its a very simple one that just calls the function.

Tests must pass

This is simple, you must make sure all automated tests are passing. If tests are failing due to your fixes, please fix your fixes. If tests are failing due to some other's changes, you must ping the project owner to make sure tests are passing.

Test Case Help

Why Add Tests?

Tests may seem like additional work, but they are very helpful for many reasons:

  1. Forces you to think of all use cases
  2. Makes the code API friendly
  3. Makes it easy for code reviews
  4. Ensures guarantee in case of future updates

User Experience (animated GIF)

If your change involves user experience, add a screenshot/narration/animated GIF. An animated GIF guarantees that you have tested your change and there are no unintended errors.

All business logic and validations must be on the server-side

If you are adding calculations or validations in your feature, they must also be on the server-side, for API users, and security.

If your contribution has a server-side change, tests must be added via on server-side

Documentation

Pull requests must involve updating the necessary documentation. Please include the link to your documentation fix in the body of your pull request.

Explanation

Include a detailed explanation for your changes, with before and after screenshots. If there is a design change, explain the use case and why this suggested change is better. If you are including a new library or replacing one, please give sufficient reference to why the suggested library is better.

Demo Script

Remember to update the demo script so that data related to your feature is included in the demo.

Migration Patches

If your design involves schema change then you must include patches that update the data as per your new schema. The patch must be added to the correct subdirectory of frappe/patches based on the version which it affects in addition to patches.txt.

Parallel Pull Requests for Bug / Security Fixes

For bug and security fixes, parallel pull requests must be sent to all supported versions. This is because older branches are no longer merged with newer branches since they have diverged to a significant level.

For example, if you fix a bug in v11 (hotfix branch), then you must also cherry-pick the commit and send another pull request on the develop branch.

Note: It is better to send backport pull request once your main pull request gets approved.

Clone this wiki locally