-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Good Pull Request
Mypy pull requests are reviewed by one of the core developers. All pull requests are not created equal, however. By following a few guidelines you can make it more likely that your PR will be merged quickly. Many of us work on mypy full-time, so you should expect an initial response to your pull request within a few days. Easy-to-merge pull requests will generally be reviewed sooner. If your pull request involves a major change, please post an issue for discussion first to avoid wasted work.
Properties of an easy-to-merge pull request:
-
PR contains a single logical change. Grouping multiple not directly related changes into a single PR makes them slower to review.
-
PR has tests (if relevant). If there are no tests, we'll probably ask you to write them. In some cases (most notably platform compatibility), we may do manual testing instead.
-
PR has clean CI builds and merges cleanly.
-
PR has a message or comments that explain what the PR does in some detail, and why this is useful. For small changes this is less important, but the more complex the change, the more important this is.
-
There is a github issue related to your PR that has been discussed, and there are no unresolved objections or open questions. This is important if your PR makes big changes or adds new dependencies. If there is an existing issue (that is not tagged as a proposal) then this is not as important, but it may still be worth it to get feedback on non-trivial design decisions. Please mention in an issue when you start work on it.
-
PRs that address github issues in more imminent milestones get priority when reviewing.
Things that may make a PR difficult or slow to review (these may even get a PR rejected, and we want to avoid wasted work):
-
The PR doesn't merge cleanly to master. Usually the creator of the PR has the most context to deal with merge conflicts. We don't get a GitHub Actions CI test build if there are merge errors.
-
There are failing tests. If you think that the failures are unrelated to your change, create a separate issue for the failure with steps to reproduce it, so that there is more visibility for others to work on a fix, and mention that this issue is blocking your PR.
-
Multiple, unrelated changes in a single PR.
-
Very large PRs (several hundred lines or more of new code). These often require manual testing and deep analysis of tradeoffs, and may also involve profiling and writing new tests. Even just reading through the code can take much time. Splitting a large PR into multiple PRs (if feasible while making each PR self-sufficient alone) can speed up code review significantly.
-
A big refactoring, rewrite or a new feature that hasn't been discussed before. This is especially true if mypy users or developers will have to change their code or workflows.
-
PR partially implements a feature or has bugs or limitations. These may or may not get merged.