[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <CUJWVSRUAJDL.2U3MGUFOREV8Q@syracuse>
Date: Fri, 04 Aug 2023 18:46:45 +0200
From: "Nicolas Escande" <nico.escande@...il.com>
To: "Ido Schimmel" <idosch@...sch.org>
Cc: <stephen@...workplumber.org>, <netdev@...r.kernel.org>
Subject: Re: [iproute2] man: bridge: update bridge link show
On Sun Jul 30, 2023 at 12:29 PM CEST, Ido Schimmel wrote:
> Thanks for the patch
>
> On Thu, Jul 27, 2023 at 07:22:08PM +0200, Nicolas Escande wrote:
> > This adds the missing [ master DEVICE ] in the synopsis part and the detailed
> > usage/effects of [ dev DEV ] & [ master DEVICE ] int the detailed syntax part
>
> Please use imperative mood [1] in the commit message. Something like:
>
> "
> Add missing man page documentation for the feature added in commit
> 13a5d8fcb41b ("bridge: link: allow filtering on bridge name").
> "
Sure I'll reference both commits that did not add man page description then.
>
> [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> >
> > Signed-off-by: Nicolas Escande <nico.escande@...il.com>
> > ---
> > man/man8/bridge.8 | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
> > index e0552819..4e7371fc 100644
> > --- a/man/man8/bridge.8
> > +++ b/man/man8/bridge.8
> > @@ -66,7 +66,10 @@ bridge \- show / manipulate bridge addresses and devices
> > .ti -8
> > .BR "bridge link" " [ " show " ] [ "
> > .B dev
> > -.IR DEV " ]"
> > +.IR DEV " ] ["
> > +.B master
> > +.IR DEVICE " ]"
> > +
> >
>
> It doesn't affect the output, but you have a double blank line here.
I'll remove it, I'm quite newbie at that man page stuff
>
> > .ti -8
> > .BR "bridge fdb" " { " add " | " append " | " del " | " replace " } "
> > @@ -661,9 +664,15 @@ display current time when using monitor option.
> >
> > .SS bridge link show - list ports configuration for all bridges.
> >
> > -This command displays port configuration and flags for all bridges.
> > +This command displays ports configuration and flags for all bridges by default.
> >
> > -To display port configuration and flags for a specific bridge, use the
> > +.TP
> > +.BI dev " DEV"
> > +only display the specific bridge port named DEV.
> > +
> > +.TP
> > +.BI master " DEVICE"
> > +only display ports of the bridge named DEVICE. This is quite similar to
>
> I would drop the "quite" and just say "similar".
ok
>
> > "ip link show master <bridge_device>" command.
> >
> > .SH bridge fdb - forwarding database management
> > --
> > 2.41.0
> >
> >
Thanks for the review, I'll send a v2 then
Powered by blists - more mailing lists