Re: [PATCH v3] doc: add new driver guidelines

[Nandini Persad <nandinipersad361@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5027 bytes --]

I like the separation. I can include it in V4 and see, it would be helpful to know if it’s more or less confusing that way.

For the prompt before each list, can we say something like “Avoid doing the following” and “Suggested actions” or something a little better grammatically. We could also just say “Avoid”.
________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Thursday, September 12, 2024 1:13:33 AM
To: Nandini Persad <nandinipersad361@gmail.com>
Cc: dev@dpdk.org <dev@dpdk.org>; Thomas Mojalon <thomas@monjalon.net>; Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v3] doc: add new driver guidelines

On 9/11/2024 5:04 PM, Nandini Persad wrote:
> Hi Ferruh,
>
> I will work with Stephen on this. For the tone of the list, I believe we
> can try different ways to make the tone more friendly, while still being
> concise.
>
> What about something like this:
> # Avoid including unused headers (process-iwyu.py).
> # Keep compiler warnings enabled in the build file.
> # Instead of using #ifdef with driver-specific macros, opt for runtime
> configuration.
> # Document device parameters in the driver guide.
> # Make device operations structs 'const'.
> # Use dynamic logging.
> # Skip DPDK version checks (RTE_VERSION_NUM) in the upstream code.
> # Add SPDX license tags and copyright notices on all sides.
> # Don’t introduce public APIs directly from the driver.
>
> It's slightly more friendly.
>
> Let me know what you think, I'm open to trying another way.
>

I think above is better.

Another option can be separating it as "Do" and "Do Not" list, as
following, do you think does it help, or makes it harder to understand?

Avoid doing:
- Using PMD specific macros when DPDK ones exist
- Including unused headers
- Disable compiler warnings for driver
- #ifdef with driver-defined macros
- DPDK version checks (via RTE_VERSION_NUM) in the upstream code
- Public APIs directly from the driver

Suggested to do:
- Runtime configuration when applicable
- Document device parameters in the driver guide
- Make device operations struct 'const'
- Dynamic logging
- SPDX license tags and copyright notice on each file


> On Tue, Sep 10, 2024 at 5:16 PM Ferruh Yigit <ferruh.yigit@amd.com
> <mailto:ferruh.yigit@amd.com>> wrote:
>
>     On 9/10/2024 3:58 PM, Nandini Persad wrote:
>     > This document was created to assist contributors
>     > in creating DPDK drivers, providing suggestions
>     > and guidelines for how to upstream effectively.
>     >
>
>     There are minor differences in this v3 and v2, isn't this version on top
>     of v2, can those changes be from Stephen?
>
>     <...>
>
>     > +
>     > +Additional Suggestions
>     > +----------------------
>     > +
>     > +* We recommend using DPDK macros instead of inventing new ones in
>     the PMD.
>     > +* Do not include unused headers (process-iwyu.py).
>     > +* Do not disable compiler warning in the build file.
>     > +* Do not use #ifdef with driver-defined macros, instead prefer
>     runtime configuration.
>     > +* Document device parameters in the driver guide.
>     > +* Make device operations struct 'const'.
>     > +* Use dynamic logging.
>     > +* Do not use DPDK version checks (via RTE_VERSION_NUM) in the
>     upstream code.
>     > +* Be sure to have SPDX license tags and copyright notice on each
>     side.
>     > +* Do not introduce public Apis directly from the driver.
>     >
>
>     API (Application Programming Interface) is an acronym and should be all
>     uppercase, like 'APIs'.
>
>     Overall the language in this list is imperative, I think it helps to
>     make it simple, but I am not sure about the tone, I wonder if we can do
>     better, do you have any suggestions?
>
>
>     > +
>     > +
>     > +Dependencies
>     > +------------
>     > +
>     > +At times, drivers may have dependencies to external software.
>     > +For driver dependencies, same DPDK rules for dependencies applies.
>     > +Dependencies should be publicly and freely available to
>     > +upstream the driver.
>     > +
>     > +
>     > +Test Tools
>     > +----------
>     > +
>     > +Per patch in a patch series, be sure to use the proper test tools.
>     > +
>     > +* checkpatches.sh
>     > +* check-git-log.sh
>     > +* check-meson.py
>     > +* check-doc-vs-code.sh
>     > +* check-spdx-tag.sh
>     >
>
>     `check-spdx-tag.sh` seems moved in v2 to "additional suggestions", I am
>     for keeping it here, as "additional suggestions" are more things to take
>     into consideration during design/development, above are actual scripts
>     that we can use to verify code.
>
>     And long term intention was to move this "tools to run list" to a more
>     generic documentation, as these are not really specific to new PMD
>     guide, but "additional suggestions" will stay in this document.
>


[-- Attachment #2: Type: text/html, Size: 7927 bytes --]