DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Conor Fogarty <conor.fogarty@intel.com>
Cc: Olivier Matz <olivier.matz@6wind.com>,
	 "Iremonger, Bernard" <bernard.iremonger@intel.com>,
	Ori Kam <orika@nvidia.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	 "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	Tomasz Kantecki <tomasz.kantecki@intel.com>,
	 Sunil Kumar Kori <skori@marvell.com>,
	Pavan Nikhilesh <pbhagavatula@marvell.com>,
	 Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Kiran Kumar Kokkilagadda <kirankumark@marvell.com>,
	 Nithin Dabilpuram <ndabilpuram@marvell.com>,
	David Hunt <david.hunt@intel.com>,
	"Burakov, Anatoly" <anatoly.burakov@intel.com>,
	 Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
	 "Singh, Jasvinder" <jasvinder.singh@intel.com>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	 Byron Marohn <byron.marohn@intel.com>,
	"Wang, Yipeng1" <yipeng1.wang@intel.com>,
	Van Haaren Harry <harry.van.haaren@intel.com>,
	Robert Sanford <rsanford@akamai.com>,
	 Erik Gabriel Carrillo <erik.g.carrillo@intel.com>,
	Declan Doherty <declan.doherty@intel.com>,
	 Kirill Rybalchenko <kirill.rybalchenko@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	 Conor Walsh <conor.walsh@intel.com>, dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] doc/sample_app_ug: use code snippets in sample app guides
Date: Thu, 15 Jul 2021 10:28:43 +0200	[thread overview]
Message-ID: <CAJFAV8xioaCnCx8c=SsBR=AQNuMjcZgVfb0txw-ZkdO9rca3Sw@mail.gmail.com> (raw)
In-Reply-To: <20210714095827.3248027-1-conor.fogarty@intel.com>

Hello,

On Wed, Jul 14, 2021 at 11:59 AM Conor Fogarty <conor.fogarty@intel.com> wrote:
>
> Currently the sample app user guides use hard coded code snippets,
> this patch changes these to use literalinclude which will dynamically
> update the snippets as changes are made to the code.
> This was introduced in commit 413c75c33c40 ("doc: show how to include
> code in guides"). Comments within the sample apps were updated to
> accommodate this as part of this patch. This will help to ensure that
> the code within the sample app user guides is up to date and not out
> of sync with the actual code.
>
> Signed-off-by: Conor Fogarty <conor.fogarty@intel.com>

Thanks for working on this cleanup.

- I see the CI reported failure on doc generation, please have a look.


- I just had a quick look at the patch, and noticed some added
"cutting" comments have been merged with existing ones, even if
unrelated.
Like:

+ /* Pattern 8< */
 existing code we want to extract
- /* Some comments unrelated to above block */
+ /* >8 End of Pattern
+  * Some comments unrelated to above block
+  */

This will just generate code churn when such unrelated blocks (and
their comments) are touched.
I would simply add /* >8 End of Pattern */ and leave existing comments
untouched.


- Did you check this patch generate the same doc outputs?


> ---
>  doc/guides/contributing/documentation.rst     |   4 +-
>  doc/guides/sample_app_ug/cmd_line.rst         |  64 +--
>  doc/guides/sample_app_ug/flow_classify.rst    | 435 +++--------------
>  doc/guides/sample_app_ug/flow_filtering.rst   | 441 ++++-------------
>  doc/guides/sample_app_ug/hello_world.rst      |  53 +-
>  doc/guides/sample_app_ug/ioat.rst             | 415 ++++------------
>  doc/guides/sample_app_ug/ip_frag.rst          |  32 +-
>  doc/guides/sample_app_ug/ip_reassembly.rst    |  67 +--
>  doc/guides/sample_app_ug/ipv4_multicast.rst   | 194 ++------
>  doc/guides/sample_app_ug/keep_alive.rst       |  38 +-
>  doc/guides/sample_app_ug/l2_forward_cat.rst   |  20 +-
>  .../sample_app_ug/l2_forward_crypto.rst       | 214 ++-------
>  doc/guides/sample_app_ug/l2_forward_event.rst | 453 ++++--------------
>  .../sample_app_ug/l2_forward_job_stats.rst    | 350 +++-----------
>  .../sample_app_ug/l2_forward_real_virtual.rst | 260 ++--------
>  doc/guides/sample_app_ug/l3_forward.rst       |  89 +---
>  doc/guides/sample_app_ug/l3_forward_graph.rst | 167 +------
>  .../sample_app_ug/l3_forward_power_man.rst    | 159 +-----
>  doc/guides/sample_app_ug/link_status_intr.rst | 267 +++--------
>  doc/guides/sample_app_ug/multi_process.rst    |  31 +-
>  doc/guides/sample_app_ug/ptpclient.rst        | 116 ++---
>  doc/guides/sample_app_ug/qos_metering.rst     |  42 +-
>  doc/guides/sample_app_ug/qos_scheduler.rst    | 143 +-----
>  doc/guides/sample_app_ug/rxtx_callbacks.rst   | 108 +----
>  doc/guides/sample_app_ug/server_node_efd.rst  | 306 ++----------
>  doc/guides/sample_app_ug/service_cores.rst    |  52 +-
>  doc/guides/sample_app_ug/skeleton.rst         | 210 ++------
>  doc/guides/sample_app_ug/timer.rst            | 133 ++---
>  .../sample_app_ug/vmdq_dcb_forwarding.rst     | 144 +-----
>  doc/guides/sample_app_ug/vmdq_forwarding.rst  | 113 +----
>  examples/cmdline/commands.c                   |   5 +-
>  examples/cmdline/main.c                       |   4 +
>  examples/flow_classify/flow_classify.c        |  60 ++-
>  examples/flow_filtering/flow_blocks.c         |  33 +-
>  examples/flow_filtering/main.c                |  35 +-
>  examples/helloworld/main.c                    |   9 +-
>  examples/ioat/ioatfwd.c                       |  57 ++-
>  examples/ip_fragmentation/main.c              |   7 +-
>  examples/ip_reassembly/main.c                 |  13 +-
>  examples/ipv4_multicast/main.c                |  43 +-
>  examples/l2fwd-cat/l2fwd-cat.c                |   8 +-
>  examples/l2fwd-crypto/main.c                  |  30 +-
>  examples/l2fwd-event/l2fwd_common.c           |  10 +-
>  examples/l2fwd-event/l2fwd_event.c            |  11 +-
>  examples/l2fwd-event/l2fwd_event_generic.c    |  11 +-
>  .../l2fwd-event/l2fwd_event_internal_port.c   |   3 +-
>  examples/l2fwd-event/l2fwd_poll.c             |  11 +-
>  examples/l2fwd-event/main.c                   |  14 +-
>  examples/l2fwd-jobstats/main.c                |  60 ++-
>  examples/l2fwd-keepalive/main.c               |   7 +-
>  examples/l2fwd/main.c                         |  36 +-
>  examples/l3fwd-graph/main.c                   |  11 +-
>  examples/l3fwd-power/main.c                   |   8 +-
>  examples/l3fwd/l3fwd_em.c                     |  10 +-
>  examples/l3fwd/l3fwd_fib.c                    |   6 +-
>  examples/l3fwd/l3fwd_lpm.c                    |   2 +
>  examples/link_status_interrupt/main.c         |  48 +-
>  examples/multi_process/simple_mp/main.c       |   2 +
>  examples/multi_process/symmetric_mp/main.c    |   4 +-
>  examples/ptpclient/ptpclient.c                |  28 +-
>  examples/qos_meter/main.c                     |  10 +-
>  examples/qos_meter/main.h                     |   4 +-
>  examples/rxtx_callbacks/main.c                |  12 +-
>  examples/server_node_efd/node/node.c          |  10 +-
>  examples/server_node_efd/server/init.c        |   5 +-
>  examples/server_node_efd/server/main.c        |  14 +-
>  examples/service_cores/main.c                 |   2 +
>  examples/skeleton/basicfwd.c                  |  30 +-
>  examples/timer/main.c                         |  23 +-
>  examples/vmdq/main.c                          |  16 +-
>  examples/vmdq_dcb/main.c                      |  10 +-
>  71 files changed, 1482 insertions(+), 4360 deletions(-)

Nice diffstat.


[snip]

> diff --git a/doc/guides/sample_app_ug/flow_classify.rst b/doc/guides/sample_app_ug/flow_classify.rst
> index 01915971ae..812aaa87b0 100644
> --- a/doc/guides/sample_app_ug/flow_classify.rst
> +++ b/doc/guides/sample_app_ug/flow_classify.rst
> @@ -64,81 +64,12 @@ ACL field definitions for the IPv4 5 tuple rule
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>  The following field definitions are used when creating the ACL table during
> -initialisation of the ``Flow Classify`` application..
> +initialisation of the ``Flow Classify`` application

It seems unrelated.
At least leave one final '.'.


>
> -.. code-block:: c
> -
> -     enum {
> -         PROTO_FIELD_IPV4,
> -         SRC_FIELD_IPV4,

[snip]

>
> --------------------------------------------------------------
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
>
>
> This e-mail and any attachments may contain confidential material for the sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact the
> sender and delete all copies.

Please get this trailer removed.


-- 
David Marchand


  reply	other threads:[~2021-07-15  8:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  9:58 Conor Fogarty
2021-07-15  8:28 ` David Marchand [this message]
2021-07-16 11:10   ` Walsh, Conor
2021-07-16 13:57 ` [dpdk-dev] [PATCH v2] " Conor Walsh
2021-07-27 10:56   ` Mcnamara, John
2021-07-31 13:46     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJFAV8xioaCnCx8c=SsBR=AQNuMjcZgVfb0txw-ZkdO9rca3Sw@mail.gmail.com' \
    --to=david.marchand@redhat.com \
    --cc=anatoly.burakov@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=byron.marohn@intel.com \
    --cc=conor.fogarty@intel.com \
    --cc=conor.walsh@intel.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=david.hunt@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=erik.g.carrillo@intel.com \
    --cc=harry.van.haaren@intel.com \
    --cc=jasvinder.singh@intel.com \
    --cc=jerinj@marvell.com \
    --cc=john.mcnamara@intel.com \
    --cc=kirankumark@marvell.com \
    --cc=kirill.rybalchenko@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=ndabilpuram@marvell.com \
    --cc=olivier.matz@6wind.com \
    --cc=orika@nvidia.com \
    --cc=pbhagavatula@marvell.com \
    --cc=rsanford@akamai.com \
    --cc=skori@marvell.com \
    --cc=thomas@monjalon.net \
    --cc=tomasz.kantecki@intel.com \
    --cc=yipeng1.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).