Re: [PATCH v2] Add CONTRIBUTING.md

[Stefano Brivio <sbrivio@redhat.com>]

Only some minor details from my side:

On Wed, 17 Sep 2025 11:51:20 +0800
Yumei Huang <yuhuang@redhat.com> wrote:

> Signed-off-by: Yumei Huang <yuhuang@redhat.com>
> ---
>  CONTRIBUTING.md | 152 ++++++++++++++++++++++++++++++++++++++++++++++++
>  README.md       |   6 +-
>  2 files changed, 156 insertions(+), 2 deletions(-)
>  create mode 100644 CONTRIBUTING.md
> 
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> new file mode 100644
> index 0000000..13fba69
> --- /dev/null
> +++ b/CONTRIBUTING.md
> @@ -0,0 +1,152 @@
> +<!---
> +SPDX-License-Identifier: GPL-2.0-or-later
> +Copyright (c) 2025 Red Hat GmbH

You don't have to assign the copyright to Red Hat GmbH (my employer),
while I tend to do that (even if things are slightly different here as
we're rather talking about authors' rights).

I guess "Red Hat" is enough in your case.

> +Author: Yumei Huang <yuhuang@redhat.com>
> +-->
> +
> +# Contributing to passt
> +
> +Thank you for your interest in contributing! This document explains how
> +to prepare patches and participate in the email-based review process.
> +
> +## Workflow
> +
> +### 1. Clone the project
> +
> +    git clone git://passt.top/passt
> +
> +

Excess newline? Or it's actually needed...?

> +### 3. Make Changes and Commit

This should be 2. now.

> +
> +* You can decide to work on the master branch or a separate branch as below:
> +
> +        cd passt
> +        git checkout -b my-feature-branch
> +
> +* Edit the source code or documentation following the [Linux kernel coding style](https://www.kernel.org/doc/html/latest/process/coding-style.html).

It won't fit into 80 columns but wrapping as much as possible is still
desirable, as it makes it more readable from smaller terminals.

> +
> +
> +* Stage your changes:
> +
> +        git add <file1> <file2> ...
> +
> +* Commit with a message.

* Commit with a message:

> +
> +        git commit
> +
> +    The message should describe your changes. See [this link](https://docs.kernel.org/process/submitting-patches.html#describe-your-changes) for details.

Same here, wrap as much as possible.

> +    Here is an example of commit message format:
> +
> +        Subsystem: Brief summary
> +
> +        More detailed explanation if needed, wrapped at 72 chars.
> +
> +    The `Subsystem` means: which part of the code your change touches.
> +    For example, it could be "tcp", "test", and "doc" etc.

Either:

  "test", or "doc", etc.

or:

  "test", "doc", etc.

but it will rarely be "test" and "doc" at the same time.

> +
> +    If there are some references, use "Links: " tag for anything.

"Links:", even if the space is then added as a separator, it's not part
of how you would call the tag.

> +
> +    Besides, passt uses the Linux kernel's "Signed-off-by" process. If you can
> +    certify the below:
> +
> +        Developer's Certificate of Origin 1.1
> +
> +        By making a contribution to this project, I certify that:
> +
> +            (a) The contribution was created in whole or in part by me and I
> +                have the right to submit it under the open source license
> +                indicated in the file; or
> +
> +            (b) The contribution is based upon previous work that, to the best
> +                of my knowledge, is covered under an appropriate open source
> +                license and I have the right under that license to submit that
> +                work with modifications, whether created in whole or in part
> +                by me, under the same open source license (unless I am
> +                permitted to submit under a different license), as indicated
> +                in the file; or
> +
> +            (c) The contribution was provided directly to me by some other
> +                person who certified (a), (b) or (c) and I have not modified
> +                it.
> +
> +            (d) I understand and agree that this project and the contribution
> +                are public and that a record of the contribution (including all
> +                personal information I submit with it, including my sign-off) is
> +                maintained indefinitely and may be redistributed consistent with
> +                this project or the open source license(s) involved.
> +
> +    Add this line:
> +
> +	    Signed-off-by: Random J Developer <random@developer.example.org>
> +
> +    using your real name (sorry, no pseudonyms or anonymous
> +    contributions.)  This will be done for you automatically if you use

...pseudonyms should actually be accepted, I think. I know there have
been discussions around this on Linux kernel mailing lists, I don't
quite remember the outcome, but I'm fairly sure that there's no
particular value in requesting a "real name" here, and it could be
problematic for some.

I'd just go with "using your name".

> +    `git commit -s`.  Reverts should also include "Signed-off-by". `git
> +    revert -s` does that for you.
> +
> +    Any further SoBs (Signed-off-by:'s) following the author's SoB are
> +    from people handling and transporting the patch, but were not involved
> +    in its development. SoB chains should reflect the **real** route a
> +    patch took as it was propagated to the maintainers, with the first SoB
> +    entry signalling primary authorship of a single author.
> +
> +### 4. Generate Patches
> +
> +Use `git format-path` to generate patch(es):

format-patch

> +
> +    git format-patch -o outgoing/ origin/master
> +
> +It will generate numbered patch files such as 0001-...patch, 0002-...patch 
> +etc. in the `outgoing` folder.
> +
> +Or you can use `git format-patch -n`. For exmaple if you want to format just

example

> +three patches:
> +
> +    git format-patch -3 -o outgoing/
> +
> +If you send a series of patches, use the `--cover-letter` option with
> +`git format-patch`:
> +
> +    git format-patch -o outgoing/ origin/main --cover-letter
> +
> +This will generate a cover letter besides your patches. You can edit the cover

s/You can edit/Edit/

...I don't think it's really optional to do so.

> +letter before sending.
> +
> +### 5. Send Patches
> +
> +Use `git send-email` to send patches directly to the mailing list:
> +
> +    git send-email --to=passt-dev@passt.top 000*.patch -o outgoing/000*.patch
> +
> +If there are CCs (e.g. maintainers, reviewers), you can add them with `--cc`:
> +
> +    git send-email --to=passt-dev@passt.top --cc=maintainer@example.com 
> +    outgoing/000*.patch
> +
> +### 6. Responding to Review Feedback
> +
> +* Be open to feedback on both code and documentation.
> +
> +* Update your patch as needed, and regenerate patches:
> +
> +        git add <file1> <file2> ...
> +        git commit --amend
> +        git format-patch -v2 origin/master
> +
> +* Send the revised patches
> +
> +        git send-email --to=passt-dev@passt.top v2-000*.patch
> +
> +### 7. Tips and Best Practices
> +
> +* Keep changes focused and easy to review. Please refer to [split-changes](https://docs.kernel.org/process/submitting-patches.html#split-changes)

Same here, wrap as much as possible.

> +  to seperate each logical change into a separate patch.
> +
> +* Test your changes thoroughly. Refer to [test/README.md](/passt/tree/test/README.md)
> +  for testing.
> +  It's recommended to run at least a 'make cppcheck' and 'make clang-tidy'
> +  other than a specific manual test of the functionality  / issue at hand.

Extra space before "/"

> +
> +* Include documentation updates if your change affects usage.
> +
> +Thank you for helping improve passt! Your contributions make a big difference.
> diff --git a/README.md b/README.md
> index 8f188f4..6ba6441 100644
> --- a/README.md
> +++ b/README.md
> @@ -553,7 +553,7 @@ See also the [test logs](/builds/latest/test/).
>          man ./passt.1
>  
>  * run the demo script, that detaches user and network namespaces, configures the
> -  new network namespace using `pasta`, starts `passt` and, optionally, `qemu`:
> +  new network namespace using `pasta`, starts `passt` *and, optionally,* `qemu`:

Spurious change I guess.

>  
>          doc/demo.sh
>  
> @@ -625,7 +625,9 @@ See also the [test logs](/builds/latest/test/).
>  
>  ### [Mailing Lists](/passt/lists)
>  * Submit, review patches, and discuss development ideas on
> -  [`passt-dev`](https://lists.passt.top/postorius/lists/passt-dev.passt.top/)
> +  [`passt-dev`](https://lists.passt.top/postorius/lists/passt-dev.passt.top/). 
> +  Please refer to the [CONTRIBUTING.md](/passt/tree/CONTRIBUTING.md) for

to the [CONTRIBUTING.md](/passt/tree/CONTRIBUTING.md) file for details.

> +  details.
>  
>  * Ask your questions and discuss usage needs on
>    [`passt-user`](https://lists.passt.top/postorius/lists/passt-user.passt.top/)

The rest looks good to me, and I think it will be really helpful this
way!

-- 
Stefano