* [Tarantool-patches] [PATCH luajit] Add contributing guidelines for Tarantool fork
@ 2023-08-03 9:16 Igor Munkin via Tarantool-patches
2023-08-03 14:08 ` Sergey Bronnikov via Tarantool-patches
2023-08-03 20:48 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 2 replies; 3+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-08-03 9:16 UTC (permalink / raw)
To: Maxim Kokryashkin, Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches
This changeset describes the particular flow of working on bug in
Tarantool LuaJIT fork. These guidelines also contains how-to for initial
working environment setup and many recomendations regarding commit
message contents.
The location of CONTRIBUING.rst is chosen to overload **all** possible
alternatives (i.e. repository root, docs/ directory, etc). For more
info, see GitHub documentation[1].
[1]: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors#adding-a-contributing-file
Signed-off-by: Igor Munkin <imun@tarantool.org>
---
Branch: https://github.com/tarantool/luajit/tree/imun/contribution-guidelines
Comments and enhancements are very welcome! I've seen that NOTE section
is misrendered by GitHub, so I have no strong opinion regarding its
usage in scope of the contributing guide.
.github/CONTRIBUTING.rst | 337 +++++++++++++++++++++++++++++++++++++++
1 file changed, 337 insertions(+)
create mode 100644 .github/CONTRIBUTING.rst
diff --git a/.github/CONTRIBUTING.rst b/.github/CONTRIBUTING.rst
new file mode 100644
index 00000000..70012468
--- /dev/null
+++ b/.github/CONTRIBUTING.rst
@@ -0,0 +1,337 @@
+.. _developer_guidelines:
+
+-------------------------------------------------------------------------------
+Developer guidelines
+-------------------------------------------------------------------------------
+
+.. _dev_guidelines-work_on_a_bug:
+
+===========================================================
+How to work on a bug
+===========================================================
+
+Any defect, even minor, if it changes the user-visible behavior, needs a bug
+report. Report a bug at https://github.com/tarantool/tarantool/issues.
+
+When reporting a bug, try to come up with a test case that can be reproduced
+with LuaJIT. Set the ``luajit`` label and label with LuaJIT subsystem affected
+if possible (e.g. ``memprof``) and the target branch for the bug fix. Assign
+the bug to yourself. Put the status to ``'In progress'``. Once the patch is
+ready, push it to your remote branch, wait for CI and fix all problems, if any
+occur. If there are test fails that look irrelevant to the changes, highlight
+this fact while emailing the patch. When CI is green, send the patch to the
+reviewers and solicit a review for the fix.
+
+Once there is a positive code review, push the patch and set the status to
+``'Done'``.
+
+Patches for bugs should contain a reference to the respective GitHub issue.
+Each patch should have a test, unless coming up with one is difficult in the
+current framework, in which case QA should be alerted.
+
+Don't forget to delete the remote branch, when your patch makes it into the
+master.
+
+.. _dev_guidelines-commit_message:
+
+===========================================================
+How to write a commit message
+===========================================================
+
+Any commit needs a helpful message. Mind the following guidelines when
+committing to any of Tarantool repositories at GitHub.
+
+1. Separate subject from body with a blank line.
+2. Try to limit the subject line to **50 characters** or so.
+3. Start the subject line with a capital letter unless it prefixed with a
+ subsystem name and semicolon:
+
+ * build:
+ * ci:
+ * cmake:
+ * core:
+ * gdb:
+ * jit:
+ * lldb:
+ * memprof:
+ * misc:
+ * sysprof:
+ * test:
+ * tools:
+ * vm:
+
+4. Do not end the subject line with a period.
+5. Do not put "gh-xx", "closes #xxx" to the subject line.
+6. Use the imperative mood in the subject line.
+ A properly formed Git commit subject line should always be able to complete
+ the following sentence: "If applied, this commit will
+ */your subject line here/*".
+7. Wrap the body to **72 characters** or so.
+8. Use the body to explain **what and why** vs. how.
+9. Link GitHub issues on the lasts lines
+ (`see how <https://help.github.com/articles/closing-issues-via-commit-messages>`_).
+10. Use your real name and real email address.
+ For Tarantool team members, **@tarantool.org** email is preferred, but not
+ mandatory.
+
+A template:
+
+.. code-block:: none
+
+ Summarize changes in 50 characters or less
+
+ More detailed explanatory text, if necessary.
+ Wrap it to 72 characters or so.
+ In some contexts, the first line is treated as the subject of the
+ commit, and the rest of the text as the body.
+ The blank line separating the summary from the body is critical
+ (unless you omit the body entirely); various tools like `log`,
+ `shortlog` and `rebase` can get confused if you run the two together.
+
+ Explain the problem that this commit is solving. Focus on why you
+ are making this change as opposed to how (the code explains that).
+ Are there side effects or other unintuitive consequences of this
+ change? Here's the place to explain them.
+
+ Further paragraphs come after blank lines.
+
+ * Bullet points are okay, too.
+
+ * Typically an asterisk or hyphen is used for the bullet, preceded
+ by a single space, with blank lines in between, but conventions
+ vary here.
+
+ Fixes tarantool/tarantool#123
+ Closes tarantool/tarantool#456
+ Needed for tarantool/tarantool#859
+ See also tarantool/tarantool#343, tarantool/tarantool#789
+
+Some real-world examples:
+
+* `tarantool/luajit@3a2e484 <https://github.com/tarantool/luajit/commit/3a2e484>`_
+* `tarantool/luajit@475359b <https://github.com/tarantool/luajit/commit/475359b>`_
+* `tarantool/luajit@47f5383 <https://github.com/tarantool/luajit/commit/47f5383>`_
+* `tarantool/luajit@4f4fd9e <https://github.com/tarantool/luajit/commit/4f4fd9e>`_
+* `tarantool/luajit@7570ff6 <https://github.com/tarantool/luajit/commit/7570ff6>`_
+* `tarantool/luajit@814625f <https://github.com/tarantool/luajit/commit/814625f>`_
+* `tarantool/luajit@88d2600 <https://github.com/tarantool/luajit/commit/88d2600>`_
+* `tarantool/luajit@9d78aa1 <https://github.com/tarantool/luajit/commit/9d78aa1>`_
+* `tarantool/luajit@a0483bd <https://github.com/tarantool/luajit/commit/a0483bd>`_
+* `tarantool/luajit@fd3f061 <https://github.com/tarantool/luajit/commit/fd3f061>`_
+
+Based on [1_] and [2_].
+
+.. _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 tarantool-patches@dev.tarantool.org. Please subscribe
+to our mailing list at https://lists.tarantool.org/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 --subject-prefix='PATCH luajit'
+
+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 --subject-prefix='PATCH luajit' -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 --subject-prefix='PATCH luajit'
+
+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 ``--subject-prefix='PATCH luajit'``, ``--cover-letter`` and
+ ``--thread=shallow`` options, you can add the following lines to
+ your gitconfig:
+
+ .. code-block:: none
+
+ [format]
+ thread = shallow
+ coverLetter = auto
+ subjectPrefix = PATCH luajit
+
+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 = your.name@gmail.com
+ 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 = your.name@mail.ru
+ 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 tarantool-patches@dev.tarantool.org 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.
+It's worth mentioning that both ``--to`` and ``--cc`` can be added on
+``git-format-patch`` step or even added to the config (it's quite convenient
+to omit ``--cc tarantool-patches@dev.tarantool.org``).
+
+.. 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.
+
+We also use git trailers in our commit messages to provide the knowledge
+about the authors and other fellows forcing the patch to appear in the
+trunk. Sometimes these guys are not actively involved in the process, so
+to avoid sending spam to any of the mentioned person just add
+``--suppress-cc=misc-by`` option to the command.
+
+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 luajit' --reroll-count=2
+
+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 Alice
+ - 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.
+
+.. _1: https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project
+.. _2: https://chris.beams.io/posts/git-commit/
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Add contributing guidelines for Tarantool fork
2023-08-03 9:16 [Tarantool-patches] [PATCH luajit] Add contributing guidelines for Tarantool fork Igor Munkin via Tarantool-patches
@ 2023-08-03 14:08 ` Sergey Bronnikov via Tarantool-patches
2023-08-03 20:48 ` Maxim Kokryashkin via Tarantool-patches
1 sibling, 0 replies; 3+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-08-03 14:08 UTC (permalink / raw)
To: Igor Munkin, Maxim Kokryashkin, Sergey Kaplun; +Cc: tarantool-patches
Hi, Igor!
Thanks for the patch! LGTM
Sergey
On 8/3/23 12:16, Igor Munkin wrote:
> This changeset describes the particular flow of working on bug in
> Tarantool LuaJIT fork. These guidelines also contains how-to for initial
> working environment setup and many recomendations regarding commit
> message contents.
>
> The location of CONTRIBUING.rst is chosen to overload **all** possible
> alternatives (i.e. repository root, docs/ directory, etc). For more
> info, see GitHub documentation[1].
>
> [1]: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors#adding-a-contributing-file
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/imun/contribution-guidelines
>
> Comments and enhancements are very welcome! I've seen that NOTE section
> is misrendered by GitHub, so I have no strong opinion regarding its
> usage in scope of the contributing guide.
>
> .github/CONTRIBUTING.rst | 337 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 337 insertions(+)
> create mode 100644 .github/CONTRIBUTING.rst
>
> diff --git a/.github/CONTRIBUTING.rst b/.github/CONTRIBUTING.rst
> new file mode 100644
> index 00000000..70012468
> --- /dev/null
> +++ b/.github/CONTRIBUTING.rst
> @@ -0,0 +1,337 @@
> +.. _developer_guidelines:
> +
> +-------------------------------------------------------------------------------
> +Developer guidelines
> +-------------------------------------------------------------------------------
> +
> +.. _dev_guidelines-work_on_a_bug:
> +
> +===========================================================
> +How to work on a bug
> +===========================================================
> +
> +Any defect, even minor, if it changes the user-visible behavior, needs a bug
> +report. Report a bug at https://github.com/tarantool/tarantool/issues.
> +
> +When reporting a bug, try to come up with a test case that can be reproduced
> +with LuaJIT. Set the ``luajit`` label and label with LuaJIT subsystem affected
> +if possible (e.g. ``memprof``) and the target branch for the bug fix. Assign
> +the bug to yourself. Put the status to ``'In progress'``. Once the patch is
> +ready, push it to your remote branch, wait for CI and fix all problems, if any
> +occur. If there are test fails that look irrelevant to the changes, highlight
> +this fact while emailing the patch. When CI is green, send the patch to the
> +reviewers and solicit a review for the fix.
> +
> +Once there is a positive code review, push the patch and set the status to
> +``'Done'``.
> +
> +Patches for bugs should contain a reference to the respective GitHub issue.
> +Each patch should have a test, unless coming up with one is difficult in the
> +current framework, in which case QA should be alerted.
> +
> +Don't forget to delete the remote branch, when your patch makes it into the
> +master.
> +
> +.. _dev_guidelines-commit_message:
> +
> +===========================================================
> +How to write a commit message
> +===========================================================
> +
> +Any commit needs a helpful message. Mind the following guidelines when
> +committing to any of Tarantool repositories at GitHub.
> +
> +1. Separate subject from body with a blank line.
> +2. Try to limit the subject line to **50 characters** or so.
> +3. Start the subject line with a capital letter unless it prefixed with a
> + subsystem name and semicolon:
> +
> + * build:
> + * ci:
> + * cmake:
> + * core:
> + * gdb:
> + * jit:
> + * lldb:
> + * memprof:
> + * misc:
> + * sysprof:
> + * test:
> + * tools:
> + * vm:
> +
> +4. Do not end the subject line with a period.
> +5. Do not put "gh-xx", "closes #xxx" to the subject line.
> +6. Use the imperative mood in the subject line.
> + A properly formed Git commit subject line should always be able to complete
> + the following sentence: "If applied, this commit will
> + */your subject line here/*".
> +7. Wrap the body to **72 characters** or so.
> +8. Use the body to explain **what and why** vs. how.
> +9. Link GitHub issues on the lasts lines
> + (`see how <https://help.github.com/articles/closing-issues-via-commit-messages>`_).
> +10. Use your real name and real email address.
> + For Tarantool team members, **@tarantool.org** email is preferred, but not
> + mandatory.
> +
> +A template:
> +
> +.. code-block:: none
> +
> + Summarize changes in 50 characters or less
> +
> + More detailed explanatory text, if necessary.
> + Wrap it to 72 characters or so.
> + In some contexts, the first line is treated as the subject of the
> + commit, and the rest of the text as the body.
> + The blank line separating the summary from the body is critical
> + (unless you omit the body entirely); various tools like `log`,
> + `shortlog` and `rebase` can get confused if you run the two together.
> +
> + Explain the problem that this commit is solving. Focus on why you
> + are making this change as opposed to how (the code explains that).
> + Are there side effects or other unintuitive consequences of this
> + change? Here's the place to explain them.
> +
> + Further paragraphs come after blank lines.
> +
> + * Bullet points are okay, too.
> +
> + * Typically an asterisk or hyphen is used for the bullet, preceded
> + by a single space, with blank lines in between, but conventions
> + vary here.
> +
> + Fixes tarantool/tarantool#123
> + Closes tarantool/tarantool#456
> + Needed for tarantool/tarantool#859
> + See also tarantool/tarantool#343, tarantool/tarantool#789
> +
> +Some real-world examples:
> +
> +* `tarantool/luajit@3a2e484 <https://github.com/tarantool/luajit/commit/3a2e484>`_
> +* `tarantool/luajit@475359b <https://github.com/tarantool/luajit/commit/475359b>`_
> +* `tarantool/luajit@47f5383 <https://github.com/tarantool/luajit/commit/47f5383>`_
> +* `tarantool/luajit@4f4fd9e <https://github.com/tarantool/luajit/commit/4f4fd9e>`_
> +* `tarantool/luajit@7570ff6 <https://github.com/tarantool/luajit/commit/7570ff6>`_
> +* `tarantool/luajit@814625f <https://github.com/tarantool/luajit/commit/814625f>`_
> +* `tarantool/luajit@88d2600 <https://github.com/tarantool/luajit/commit/88d2600>`_
> +* `tarantool/luajit@9d78aa1 <https://github.com/tarantool/luajit/commit/9d78aa1>`_
> +* `tarantool/luajit@a0483bd <https://github.com/tarantool/luajit/commit/a0483bd>`_
> +* `tarantool/luajit@fd3f061 <https://github.com/tarantool/luajit/commit/fd3f061>`_
> +
> +Based on [1_] and [2_].
> +
> +.. _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 tarantool-patches@dev.tarantool.org. Please subscribe
> +to our mailing list at https://lists.tarantool.org/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 --subject-prefix='PATCH luajit'
> +
> +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 --subject-prefix='PATCH luajit' -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 --subject-prefix='PATCH luajit'
> +
> +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 ``--subject-prefix='PATCH luajit'``, ``--cover-letter`` and
> + ``--thread=shallow`` options, you can add the following lines to
> + your gitconfig:
> +
> + .. code-block:: none
> +
> + [format]
> + thread = shallow
> + coverLetter = auto
> + subjectPrefix = PATCH luajit
> +
> +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 = your.name@gmail.com
> + 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 = your.name@mail.ru
> + 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 tarantool-patches@dev.tarantool.org 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.
> +It's worth mentioning that both ``--to`` and ``--cc`` can be added on
> +``git-format-patch`` step or even added to the config (it's quite convenient
> +to omit ``--cc tarantool-patches@dev.tarantool.org``).
> +
> +.. 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.
> +
> +We also use git trailers in our commit messages to provide the knowledge
> +about the authors and other fellows forcing the patch to appear in the
> +trunk. Sometimes these guys are not actively involved in the process, so
> +to avoid sending spam to any of the mentioned person just add
> +``--suppress-cc=misc-by`` option to the command.
> +
> +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 luajit' --reroll-count=2
> +
> +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 Alice
> + - 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.
> +
> +.. _1: https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project
> +.. _2: https://chris.beams.io/posts/git-commit/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Add contributing guidelines for Tarantool fork
2023-08-03 9:16 [Tarantool-patches] [PATCH luajit] Add contributing guidelines for Tarantool fork Igor Munkin via Tarantool-patches
2023-08-03 14:08 ` Sergey Bronnikov via Tarantool-patches
@ 2023-08-03 20:48 ` Maxim Kokryashkin via Tarantool-patches
1 sibling, 0 replies; 3+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-08-03 20:48 UTC (permalink / raw)
To: Igor Munkin; +Cc: tarantool-patches
Hi, Igor!
Thanks for the guidelines!
On Thu, Aug 03, 2023 at 09:16:07AM +0000, Igor Munkin wrote:
> This changeset describes the particular flow of working on bug in
Typo: s/on bug/on a bug/
> Tarantool LuaJIT fork. These guidelines also contains how-to for initial
Typo: s/Tarantool/the Tarantool/
Typo: s/also contains/also contain a/
> working environment setup and many recomendations regarding commit
Typo: s/recomendations/recommendations/
> message contents.
Typo: s/regarding commit message contents/regarding the contents of commit messages/
>
> The location of CONTRIBUING.rst is chosen to overload **all** possible
Typo: s/CONTRIBUING.rst/CONTRIBUTING.rst/
> alternatives (i.e. repository root, docs/ directory, etc). For more
> info, see GitHub documentation[1].
>
> [1]: https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors#adding-a-contributing-file
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/imun/contribution-guidelines
>
> Comments and enhancements are very welcome! I've seen that NOTE section
> is misrendered by GitHub, so I have no strong opinion regarding its
> usage in scope of the contributing guide.
Some general thoughts:
- I thinks we should add something about checking the spelling, gramma,
phrasing and abscence of profanity in the patch.
- I completely understand that this guide is targeted at an imaginary
external contributor, but it's unlikely someone other then a member of
our team will do something. So maybe we should add something about our CI?
- Also, it is important to mention, that the whole test set must be executed
before sending the patch anywhere.
- Not sure about the "favorite mailing client" thing. For example, I used to
send my reviews via the web interface, which resulted in incorrectly displayed
interleaved comments for everyone else. So, IMO, at least a list of some good
options (Thunderbird, [neo]mutt, etc.) should be provided.
>
> .github/CONTRIBUTING.rst | 337 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 337 insertions(+)
> create mode 100644 .github/CONTRIBUTING.rst
>
> diff --git a/.github/CONTRIBUTING.rst b/.github/CONTRIBUTING.rst
> new file mode 100644
> index 00000000..70012468
> --- /dev/null
> +++ b/.github/CONTRIBUTING.rst
> @@ -0,0 +1,337 @@
> +.. _developer_guidelines:
> +
> +-------------------------------------------------------------------------------
> +Developer guidelines
> +-------------------------------------------------------------------------------
> +
> +.. _dev_guidelines-work_on_a_bug:
> +
> +===========================================================
> +How to work on a bug
> +===========================================================
> +
> +Any defect, even minor, if it changes the user-visible behavior, needs a bug
Typo: s/even minor/even a minor one/
> +report. Report a bug at https://github.com/tarantool/tarantool/issues.
> +
> +When reporting a bug, try to come up with a test case that can be reproduced
> +with LuaJIT. Set the ``luajit`` label and label with LuaJIT subsystem affected
Typo: s/with/with the/
> +if possible (e.g. ``memprof``) and the target branch for the bug fix. Assign
> +the bug to yourself. Put the status to ``'In progress'``. Once the patch is
> +ready, push it to your remote branch, wait for CI and fix all problems, if any
> +occur. If there are test fails that look irrelevant to the changes, highlight
> +this fact while emailing the patch. When CI is green, send the patch to the
> +reviewers and solicit a review for the fix.
> +
> +Once there is a positive code review, push the patch and set the status to
> +``'Done'``.
> +
> +Patches for bugs should contain a reference to the respective GitHub issue.
> +Each patch should have a test, unless coming up with one is difficult in the
> +current framework, in which case QA should be alerted.
> +
> +Don't forget to delete the remote branch, when your patch makes it into the
> +master.
> +
> +.. _dev_guidelines-commit_message:
> +
> +===========================================================
> +How to write a commit message
> +===========================================================
> +
> +Any commit needs a helpful message. Mind the following guidelines when
> +committing to any of Tarantool repositories at GitHub.
Typo: s/of/of the/
> +
> +1. Separate subject from body with a blank line.
Typo: s/subject from body/the subject from the body/
> +2. Try to limit the subject line to **50 characters** or so.
> +3. Start the subject line with a capital letter unless it prefixed with a
Typo: s/it/it is/
> + subsystem name and semicolon:
Typo: s/and/and a/
> +
> + * build:
> + * ci:
> + * cmake:
> + * core:
> + * gdb:
> + * jit:
> + * lldb:
> + * memprof:
> + * misc:
> + * sysprof:
> + * test:
> + * tools:
> + * vm:
> +
> +4. Do not end the subject line with a period.
> +5. Do not put "gh-xx", "closes #xxx" to the subject line.
> +6. Use the imperative mood in the subject line.
> + A properly formed Git commit subject line should always be able to complete
> + the following sentence: "If applied, this commit will
> + */your subject line here/*".
> +7. Wrap the body to **72 characters** or so.
> +8. Use the body to explain **what and why** vs. how.
> +9. Link GitHub issues on the lasts lines
Typo: s/lasts/last/
> + (`see how <https://help.github.com/articles/closing-issues-via-commit-messages>`_).
> +10. Use your real name and real email address.
> + For Tarantool team members, **@tarantool.org** email is preferred, but not
> + mandatory.
Side note: I believe you should also specify, that some of those requirements
can be ignored for backported patches.
> +
> +A template:
> +
> +.. code-block:: none
> +
> + Summarize changes in 50 characters or less
> +
> + More detailed explanatory text, if necessary.
> + Wrap it to 72 characters or so.
> + In some contexts, the first line is treated as the subject of the
> + commit, and the rest of the text as the body.
> + The blank line separating the summary from the body is critical
> + (unless you omit the body entirely); various tools like `log`,
> + `shortlog` and `rebase` can get confused if you run the two together.
> +
> + Explain the problem that this commit is solving. Focus on why you
> + are making this change as opposed to how (the code explains that).
> + Are there side effects or other unintuitive consequences of this
> + change? Here's the place to explain them.
> +
> + Further paragraphs come after blank lines.
> +
> + * Bullet points are okay, too.
> +
> + * Typically an asterisk or hyphen is used for the bullet, preceded
> + by a single space, with blank lines in between, but conventions
> + vary here.
> +
> + Fixes tarantool/tarantool#123
> + Closes tarantool/tarantool#456
> + Needed for tarantool/tarantool#859
> + See also tarantool/tarantool#343, tarantool/tarantool#789
> +
> +Some real-world examples:
> +
> +* `tarantool/luajit@3a2e484 <https://github.com/tarantool/luajit/commit/3a2e484>`_
> +* `tarantool/luajit@475359b <https://github.com/tarantool/luajit/commit/475359b>`_
> +* `tarantool/luajit@47f5383 <https://github.com/tarantool/luajit/commit/47f5383>`_
> +* `tarantool/luajit@4f4fd9e <https://github.com/tarantool/luajit/commit/4f4fd9e>`_
> +* `tarantool/luajit@7570ff6 <https://github.com/tarantool/luajit/commit/7570ff6>`_
> +* `tarantool/luajit@814625f <https://github.com/tarantool/luajit/commit/814625f>`_
> +* `tarantool/luajit@88d2600 <https://github.com/tarantool/luajit/commit/88d2600>`_
> +* `tarantool/luajit@9d78aa1 <https://github.com/tarantool/luajit/commit/9d78aa1>`_
> +* `tarantool/luajit@a0483bd <https://github.com/tarantool/luajit/commit/a0483bd>`_
> +* `tarantool/luajit@fd3f061 <https://github.com/tarantool/luajit/commit/fd3f061>`_
> +
> +Based on [1_] and [2_].
> +
> +.. _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 tarantool-patches@dev.tarantool.org. Please subscribe
> +to our mailing list at https://lists.tarantool.org/tarantool-patches to ensure
> +that your messages are added to the archive.
> +
> +1. **Preparing a patch**
Nit: Further contents of this bullet are under-indented on the GitHun. I'd be nice
to fix it or just make it a subsection, but feel free to ignore.
> +
> +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:
Typo: s/use/use the/
> +
> +.. code-block:: console
> +
> + $ git format-patch -1 --subject-prefix='PATCH luajit'
> +
> +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:
Typo: s/use/use the/
> +
> +.. code-block:: console
> +
> + $ git format-patch -1 --subject-prefix='PATCH luajit' -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 --subject-prefix='PATCH luajit'
> +
> +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):
Typo: s/via/via the/
> +
> +.. 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::
Nit: Maybe just a bold text will do?
> +
> + To omit ``--subject-prefix='PATCH luajit'``, ``--cover-letter`` and
> + ``--thread=shallow`` options, you can add the following lines to
> + your gitconfig:
> +
> + .. code-block:: none
> +
> + [format]
> + thread = shallow
> + coverLetter = auto
> + subjectPrefix = PATCH luajit
> +
> +2. **Sending a patch**
Same as for the bullet above.
> +
> +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 = your.name@gmail.com
> + 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 = your.name@mail.ru
> + smtppass = topsecret
Side note: Let's go in and out. Quick, 20-minute adventure.
> +
> +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 tarantool-patches@dev.tarantool.org 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.
It is not necessary to do that for each recipient. You can just do this:
| --to=tarantool-patches@dev.tarantool.org,colleague1@tarantool.org,colleague2@tarantool.org
Also, I guess that 'if you want someone in particular to review your patch' is kind of
misleading. I doubt anyone would review a patch, that was not CC'ed to them.
> +It's worth mentioning that both ``--to`` and ``--cc`` can be added on
> +``git-format-patch`` step or even added to the config (it's quite convenient
> +to omit ``--cc tarantool-patches@dev.tarantool.org``).
> +
> +.. 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.
Side note: Maybe it is worth mentioning that for the backported patches, Mike's
stub <mike> should be dropped during the sending process from the list of recipients.
> +
> +We also use git trailers in our commit messages to provide the knowledge
> +about the authors and other fellows forcing the patch to appear in the
> +trunk. Sometimes these guys are not actively involved in the process, so
> +to avoid sending spam to any of the mentioned person just add
> +``--suppress-cc=misc-by`` option to the command.
> +
> +3. **Review process**
Same as for the bullet above.
> +
> +After having sent your patches, you just wait for a review. The reviewer
Typo: s/having sent/sending/
> +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
Typo: s/to email/to your email/
> +favorite mail agent, or use ``--in-reply-to`` option of ``git send-email``
Typo: s/use/use the/
Typo: s/of/of the/
> +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:
Typo: s/to/to the/
> +
> +.. code-block:: console
> +
> + $ git format-patch -1 --subject-prefix='PATCH luajit' --reroll-count=2
> +
> +To add a change log, open the generated email with you favorite text
Typo: s/you/your/
> +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 Alice
> + - 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.
> +
> +.. _1: https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project
> +.. _2: https://chris.beams.io/posts/git-commit/
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-03 20:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03 9:16 [Tarantool-patches] [PATCH luajit] Add contributing guidelines for Tarantool fork Igor Munkin via Tarantool-patches
2023-08-03 14:08 ` Sergey Bronnikov via Tarantool-patches
2023-08-03 20:48 ` Maxim Kokryashkin via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox