From: Petr Machata <petrm@nvidia•com>
To: Daniel Machon <daniel.machon@microchip•com>
Cc: Petr Machata <petrm@nvidia•com>, <netdev@vger•kernel.org>,
<dsahern@kernel•org>, <stephen@networkplumber•org>,
<UNGLinuxDriver@microchip•com>
Subject: Re: [PATCH iproute2-next v2 2/8] dcb: app: modify dcb-app print functions for dcb-rewr reuse
Date: Wed, 31 May 2023 13:26:06 +0200 [thread overview]
Message-ID: <87o7m04tq5.fsf@nvidia.com> (raw)
In-Reply-To: <20230531083141.ijtwsfxa3javczdf@DEN-LT-70577>
Daniel Machon <daniel.machon@microchip•com> writes:
>> Daniel Machon <daniel.machon@microchip•com> writes:
>>
>> > Where dcb-app requires protocol to be the printed key, dcb-rewr requires
>> > it to be the priority. Adapt existing dcb-app print functions for this.
>> >
>> > dcb_app_print_filtered() has been modified, to take two callbacks; one
>> > for printing the entire string (pid and prio), and one for the pid type
>> > (dec, hex, dscp, pcp). This saves us for making one dedicated function
>> > for each pid type for both app and rewr.
>> >
>> > dcb_app_print_key_*() functions have been renamed to
>> > dcb_app_print_pid_*() to align with new situation. Also, none of them
>> > will print the colon anymore.
>> >
>> > Signed-off-by: Daniel Machon <daniel.machon@microchip•com>
>>
>> There are about four patches included in this one patch: the %d->%u
>> change, the colon shenanigans, the renaming, and prototype change of
>> dcb_app_print_filtered().
>>
>> I think the code is OK, but I would appreciate splitting into a patch
>> per feature.
>
> Actually, IMHO, the 'colon shenanigans' does not make much sense in a
> patch of its own, since its not really a standalone feature, but rather
> some change that is required for the prototype change of
> dcb_app_print_filtered?
It doesn't make sense on its own, but it also does not get sent on its
own, but together with the following patches. Its role is to set stage.
dcb_app_print_filtered() should first stop assuming the callback prints
a colon. So you have one patch where the colon moves from the callback
to _print_filtered(). That's a clean, behavior-neutral change that
should be trivial to review. Then next patch introduces the callback.
Which at that point should likewise be tidy, focused and easy to review,
because it will only deal with the new callback.
next prev parent reply other threads:[~2023-05-31 11:38 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-25 18:10 [PATCH iproute2-next v2 0/8] Introduce new dcb-rewr subcommand Daniel Machon
2023-05-25 18:10 ` [PATCH iproute2-next v2 1/8] dcb: app: add new dcbnl attribute field Daniel Machon
2023-05-25 18:10 ` [PATCH iproute2-next v2 2/8] dcb: app: modify dcb-app print functions for dcb-rewr reuse Daniel Machon
2023-05-29 17:09 ` Petr Machata
2023-05-30 8:01 ` Daniel Machon
2023-05-31 8:31 ` Daniel Machon
2023-05-31 11:26 ` Petr Machata [this message]
2023-05-31 14:28 ` David Ahern
2023-05-25 18:10 ` [PATCH iproute2-next v2 3/8] dcb: app: modify dcb_app_table_remove_replaced() " Daniel Machon
2023-05-29 17:00 ` Petr Machata
2023-05-30 8:03 ` Daniel Machon
2023-05-30 20:29 ` Petr Machata
2023-05-25 18:10 ` [PATCH iproute2-next v2 4/8] dcb: app: modify dcb_app_parse_mapping_cb " Daniel Machon
2023-05-30 19:50 ` Petr Machata
2023-05-31 8:12 ` Daniel Machon
2023-05-31 11:05 ` Petr Machata
2023-05-25 18:10 ` [PATCH iproute2-next v2 5/8] dcb: rewr: add new dcb-rewr subcommand Daniel Machon
2023-05-30 19:58 ` Petr Machata
2023-05-31 8:14 ` Daniel Machon
2023-05-25 18:10 ` [PATCH iproute2-next v2 6/8] man: dcb-rewr: add new manpage for dcb-rewr Daniel Machon
2023-05-30 19:47 ` Petr Machata
2023-05-25 18:10 ` [PATCH iproute2-next v2 7/8] man: dcb: add additional references under 'SEE ALSO' Daniel Machon
2023-05-30 19:39 ` Petr Machata
2023-05-25 18:10 ` [PATCH iproute2-next v2 8/8] man: dcb-app: clean up a few mistakes Daniel Machon
2023-05-30 19:37 ` Petr Machata
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=87o7m04tq5.fsf@nvidia.com \
--to=petrm@nvidia$(echo .)com \
--cc=UNGLinuxDriver@microchip$(echo .)com \
--cc=daniel.machon@microchip$(echo .)com \
--cc=dsahern@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=stephen@networkplumber$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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