Skip to content

Delete 'How to submit a patch for review' section from dev guide #2877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 0 additions & 203 deletions doc/dev_guide/developer_guidelines.rst
Original file line number Diff line number Diff line change
Expand Up @@ -109,206 +109,3 @@ Some real-world examples:

Based on [`1 <https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project>`_]
and [`2 <https://chris.beams.io/posts/git-commit/>`_].

.. _dev_guidelines-patch-review:

===========================================================
How to submit a patch for review
===========================================================

We don't accept GitHub pull requests. Instead, all patches
should be sent as plain-text messages to [email protected].
Please subscribe to our mailing list
at https://lists.tarantool.org/mailman/listinfo/tarantool-patches
to ensure that your messages are added to the archive.

1. **Preparing a patch**

Once you have committed a patch to your local git repository, you can
submit it for review.

To prepare an email, use ``git format-patch`` command:

.. code-block:: console

$ git format-patch -1

It will format the commit at the top of your local git repository as
a plain-text email and write it to a file in the current directory.
The file name will look like ``0001-your-commit-subject-line.patch``.
To specify a different directory, use ``-o`` option:

.. code-block:: console

$ git format-patch -1 -o ~/patches-to-send

Once the patch has been formatted, you can view and edit it with your
favorite text editor (after all, it is a plain-text file!). We strongly
recommend adding:

* a hyperlink to the branch where this patch can be found at GitHub, and
* a hyperlink to the GitHub issue your patch is supposed to fix, if any.

If there is just one patch, the change log should go right after ``---`` in the
message body (it will be ignored by ``git am`` then).

If there are multiple patches you want to submit in one go (e.g. this is
a big feature which requires some preparatory patches to be committed
first), you should send each patch in a separate email in reply to a cover
letter. To format a patch series accordingly, pass the following options
to ``git format-patch``:

.. code-block:: console

$ git format-patch --cover-letter --thread=shallow HEAD~2

where:

* ``--cover-letter`` will make ``git format-patch`` generate a cover letter;
* ``--thread=shallow`` will mark each formatted patch email to be sent
in reply to the cover letter;
* ``HEAD~2`` (we now use it instead of ``-1``) will make ``git format-patch``
format the first two patches at the top of your local git branch instead
of just one. To format three patches, use ``HEAD~3``, and so forth.

After the command has been successfully executed, you will find all your
patches formatted as separate emails in your current directory (or in the
directory specified via ``-o`` option):

.. code-block:: none

0000-cover-letter.patch
0001-first-commit.patch
0002-second-commit.patch
...

The cover letter will have BLURB in its subject and body. You'll have to
edit it before submitting (again, it is a plain text file). Please write:

* a short series description in the subject line;
* a few words about each patch of the series in the body.

And don't forget to add hyperlinks to the GitHub issue and branch where
your series can be found. In this case you don't need to put links or any
additional information to each individual email -- the cover letter will
cover everything.

.. NOTE::

To omit ``--cover-letter`` and ``--thread=shallow`` options, you can
add the following lines to your gitconfig:

.. code-block:: none

[format]
thread = shallow
coverLetter = auto

2. **Sending a patch**

Once you have formatted your patches, they are ready to be sent via email.
Of course, you can send them with your favorite mail agent, but it is
much easier to use ``git send-email`` for this. Before using this command,
you need to configure it.

If you use a GMail account, add the following code to your ``.gitconfig``:

.. code-block:: none

[sendemail]
smtpencryption = tls
smtpserver = smtp.gmail.com
smtpserverport = 587
smtpuser = [email protected]
smtppass = topsecret

For mail.ru users, the configuration will be slightly different:

.. code-block:: none

[sendemail]
smtpencryption = ssl
smtpserver = smtp.mail.ru
smtpserverport = 465
smtpuser = [email protected]
smtppass = topsecret

If your email account is hosted by another service, consult your service
provider about your SMTP settings.

Once configured, use the following command to send your patches:

.. code-block:: console

$ git send-email --to [email protected] 00*

(``00*`` wildcard will be expanded by your shell to the list of patches
generated at the previous step.)

If you want someone in particular to review your patch, add them to the
list of recipients by passing ``--to`` or ``--cc`` once per each recipient.

.. NOTE::

It is useful to check that ``git send-email`` will work as expected
without sending anything to the world. Use ``--dry-run`` option for that.

3. **Review process**

After having sent your patches, you just wait for a review. The reviewer
will send their comments back to you in reply to the email that contains
the patch that in their opinion needs to be fixed.

Upon receiving an email with review remarks, you carefully read it and reply
about whether you agree or disagree with. Please note that we use the
interleaved reply style (aka "inline reply") for communications over email.

Upon reaching an agreement, you send a fixed patch in reply to the email that
ended the discussion. To send a patch, you can either attach a plain diff
(created by ``git diff`` or ``git format-patch``) to email and send it with your
favorite mail agent, or use ``--in-reply-to`` option of ``git send-email``
command.

If you feel that the accumulated change set is large enough to send the
whole series anew and restart the review process in a different thread,
you generate the patch email(s) again with ``git format-patch``, this time
adding v2 (then v3, v4, and so forth) to the subject and a change log to
the message body. To modify the subject line accordingly, use the
``--subject-prefix`` option to ``git format-patch`` command:

.. code-block:: console

$ git format-patch -1 --subject-prefix='PATCH v2'

To add a change log, open the generated email with you favorite text
editor and edit the message body. If there is just one patch, the change
log should go right after ``---`` in the message body (it will be ignored
by ``git am`` then). If there is more than one patch, the change log should
be added to the cover letter. Here is an example of a good change log:

.. code-block:: console

Changes in v3:
- Fixed comments as per review by Alex
- Added more tests
Changes in v2:
- Fixed a crash if the user passes invalid options
- Fixed a memory leak at exit

It is also a good practice to add a reference to the previous version of
your patch set (via a hyperlink or message id).

.. NOTE::

* Do not disagree with the reviewer without providing a good argument
supporting your point of view.
* Do not take every word the reviewer says for granted. Reviewers are
humans too, hence fallible.
* Do not expect that the reviewer will tell you how to do your thing.
It is not their job. The reviewer might suggest alternative ways to
tackle the problem, but in general it is your responsibility.
* Do not forget to update your remote git branch every time you send a
new version of your patch.
* Do follow the guidelines above. If you do not comply, your patches are
likely to be silently ignored.

Loading