From 558e74af3973f73e81738b27bcb327e4ce2c1615 Mon Sep 17 00:00:00 2001 From: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> Date: Sat, 21 Sep 2019 23:20:28 +0200 Subject: [PATCH] Update PR guidelines --- doc/devel/coding_guide.rst | 164 ++++++++++++++++++++++++++++++------- 1 file changed, 133 insertions(+), 31 deletions(-) diff --git a/doc/devel/coding_guide.rst b/doc/devel/coding_guide.rst index 49e77b7da6f0..422c271a15e8 100644 --- a/doc/devel/coding_guide.rst +++ b/doc/devel/coding_guide.rst @@ -1,20 +1,73 @@ -.. _reviewers-guide: +.. _pr-guidelines: -******************** -Reviewers guideline -******************** +*********************** +Pull request guidelines +*********************** -.. _pull-request-checklist: +Pull requests (PRs) are the mechanism for contributing to Matplotlibs code and +documentation. -Pull request checklist +Summary for PR authors ====================== -Branch selection ----------------- +.. note:: + + * We value contributions from people with all levels of experience. In + particular if this is your first PR not everything has to be perfect. + We'll guide you through the PR process. + * Nevertheless, try to follow the guidelines below as well as you can to + help make the PR process quick and smooth. + * Be patient with reviewers. We try our best to respond quickly, but we + have limited bandwidth. If there is no feedback within a couple of days, + please ping us by posting a comment to your PR. + +When making a PR, pay attention to: + +* :ref:`Target the master branch `. +* Adhere to the :ref:`coding_guidelines`. +* Update the :ref:`documentation ` if necessary. +* Aim at making the PR as "ready-to-go" as you can. This helps to speed up + the review process. +* It is ok to open incomplete or work-in-progress PRs if you need help or + feedback from the developers. You may mark these as + `draft pull requests `_ + on GitHub. + +See also :ref:`contributing` for how to make a PR. + +Summary for PR reviewers +======================== + +.. note:: + + * If you have commit rights, then you are trusted to use them. + **Please help review and merge PRs!** + * Be patient and `kind `__ with + contributors. + +Content topics: -Generally, all pull requests (*PRs*) should target the master branch. Other -branches are fed through automatic or manual backports. Directly targeting -other branches is only rarely necessary for special maintenance work. +* Is the feature / bugfix reasonable? +* Does the PR conform with the :ref:`coding_guidelines`? +* Is the :ref:`documentation ` (docstrings, examples, + what's new, API changes) updated? + +Organizational topics: + +* Make sure all :ref:`automated tests ` pass. +* The PR should :ref:`target the master branch `. +* Tag with descriptive :ref:`labels `. +* Set the :ref:`milestone `. +* Keep an eye on the :ref:`number of commits `. +* Approve if all of the above topics handled. +* :ref:`Merge ` if a sufficient number of approvals is reached. + +.. _pr-guidelines-details: + +Detailed Guidelines +=================== + +.. _pr-documentation: Documentation ------------- @@ -38,24 +91,25 @@ Documentation * If you change the API in a backward-incompatible way, please document it in :file:`doc/api/api_changes.rst`. -PR Review guidelines -==================== +.. _pr-labels: -* Be patient and `kind `__ with - contributors. - -* If you have commit rights, then you are trusted to use them. Please - help review and merge PRs! +Labels +------ * If you have the rights to set labels, tag the PR with descriptive labels. See the `list of labels `__. +.. _pr-milestones: + +Milestones +---------- + * Set the milestone according to these rules: - * *New features and API changes* are milestoned for the next point-release + * *New features and API changes* are milestoned for the next minor release ``v3.X.0``. - * *Bugfixes and docstring changes* are milestoned for the next bugfix + * *Bugfixes and docstring changes* are milestoned for the next patch release ``v3.X.Y`` * *Documentation changes* (all .rst files and examples) are milestoned @@ -67,6 +121,11 @@ PR Review guidelines an :ref:`automatic backport ` for milestones which have a corresponding branch. +.. _pr-merging: + +Merging +------- + * Documentation and examples may be merged by the first reviewer. Use the threshold "is this better than it was?" as the review criteria. @@ -95,6 +154,11 @@ PR Review guidelines A core dev should only champion one PR at a time and we should try to keep the flow of championed PRs reasonable. +.. _pr-automated-tests: + +Automated tests +--------------- + * Make sure the Travis, Appveyor, CircleCI, and codecov tests are passing before merging. @@ -107,6 +171,11 @@ PR Review guidelines when another reviewer explicitly allows it (ex, "Approve modulo CI passing, may self merge when green"). +.. _pr-squashing: + +Number of commits and squashing +------------------------------- + * Squashing is case-by-case. The balance is between burden on the contributor, keeping a relatively clean history, and keeping a history usable for bisecting. The only time we are really strict @@ -126,37 +195,68 @@ PR Review guidelines with the contributor first. +.. _branches_and_backports: + Branches and Backports ====================== - +Current branches +---------------- The current active branches are *master* - This will be Matplotlib 3.0. Supports Python 3.5+. + The current development version. Future minor releases (*v3.N.0*) will be + branched from this. Supports Python 3.6+. + +*v3.N.x* + Maintenance branch for Matplotlib 3.N. Future patch releases will be + branched from this. Supports Python 3.6+. + +*v3.N.M-doc* + Documentation for the current release. On a patch release, this will be + replaced by a properly named branch for the new release. *v2.2.x* - Maintenance branch for Matplotlib 2.2 LTS. Supports Python 2.7, 3.4+ + Maintenance branch for Matplotlib 2.2 LTS. Supports Python 2.7, 3.4+. *v2.2.N-doc* - Documentation for the current release. On a patch release, this will be replaced - by a properly named branch for the new release. + Documentation for the current release. On a patch release, this will be + replaced by a properly named branch for the new release. + + +.. _pr-branch-selection: + +Branch selection for pull requests +---------------------------------- + +Generally, all pull requests should target the master branch. +Other branches are fed through :ref:`automatic ` or +:ref:`manual `. Directly +targeting other branches is only rarely necessary for special maintenance +work. -We always will backport to 2.2.x +.. backport_strategy: + +Backport strategy +----------------- + +We will always backport to the patch release branch (*v3.N.x*): - critical bug fixes (segfault, failure to import, things that the user can not work around) -- fixes for regressions against 2.0 or 2.1 +- fixes for regressions against the last two releases. -Everything else (regressions against 1.x versions, bugs/api +Everything else (regressions against older releases, bugs/api inconsistencies the user can work around in their code) are on a case-by-case basis, should be low-risk, and need someone to advocate for and shepherd through the backport. -The only changes to be backported to 2.2.N-doc are changes to -``doc``, ``examples``, or ``tutorials``. Any changes to -``lib`` or ``src`` should not be backported to this branch. +The only changes to be backported to the documentation branch (*v3.N.M-doc*) +are changes to :file:`doc`, :file:`examples`, or :file:`tutorials`. +Any changes to :file:`lib` or :file:`src` including docstring-only changes +should not be backported to this branch. + .. _automated-backports: @@ -178,6 +278,8 @@ If the bot is not working as expected, please report issues to `Meeseeksdev `__. +.. _manual-backports: + Manual backports ----------------