netdev - Re: [iproute2] man: bridge: update bridge link show

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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