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

[Nandini Persad <nandinipersad361@gmail.com>]

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

Excellent. Will send the update in accordingly.
________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Thursday, September 12, 2024 6:37:35 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/12/2024 2:18 PM, Nandini Persad wrote:
> 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”.
>

Agree to have better header for the lists, what about:
"Avoid doing the following" and
"Remember to do the following"

Or we can go with whatever you think more convenient.

> ------------------------------------------------------------------------
> *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 <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: 9344 bytes --]