Re: Yangdoctors early review of draft-ietf-bfd-unsolicited-01

[Martin Bjorklund <[e-mail]>]

"Reshad Rahman (rrahman)" <[e-mail]> wrote:
> Thanks Martin and Mahesh.
> 
> I believe we should add a mandatory statement to the choic (speaking
> as BFD YANG co-author,)

But then it is not clear why all leafs in the cases have default
statements.

Since the 'single-interval' case is optional with a if-feature (which
BTW is weird since it is trivial to implement), and the only other
case has default values on both its leafs, I would have assumed that
the intention was that if nothing is configured, the server should use
1000000 microseconds for the intervals.  If this is the intention,
perhaps a statement:  "default tx-rx-intervals;" can be added to the
module, even though the doc is in the RFC ed q.


/martin



> 
> Just created https://github.com/bfd-wg
> 
> Regards,
> Reshad.
> 
> 
> On 2019-08-19, 2:45 PM, "Mahesh Jethanandani" <[e-mail]> wrote:
> 
>     [Adding the authors of BFD YANG module]
>     
>     Martin brings up a good point. But since the document that contains ietf-bfd-types is sitting in RFC Ed Queue, this will have to go into a bis document.
>     
>     Chairs, could you create a bfd-wg in GitHub for us to track this as an issue to be fixed as part of a bis document?
>     
>     > On Aug 19, 2019, at 4:29 AM, Martin Björklund via Datatracker <[e-mail]> wrote:
>     > 
>     > Reviewer: Martin Björklund
>     > Review result: Ready with Nits
>     > 
>     > I have reviewed this document from a YANG model perspective only.
>     > 
>     > My only comment is actually for a grouping defined in ietf-bfd-type, but used
>     > in this module.  There is a choice "interval-config-type":
>     > 
>     >  +--rw unsolicited {bfd-unsol:unsolicited-params-global}?
>     >       +--rw enable?                           boolean
>     >       +--rw local-multiplier?                 multiplier
>     >       +--rw (interval-config-type)?
>     >          +--:(tx-rx-intervals)
>     >          |  +--rw desired-min-tx-interval?    uint32
>     >          |  +--rw required-min-rx-interval?   uint32
>     >          +--:(single-interval) {single-minimum-interval}?
>     >             +--rw min-interval?               uint32
>     > 
>     > This choice is not mandatory and doesn't have a default case, so the question
>     > is what happens if no nodes from the choice has been configured?   I would
>     > expect the choice to have a default case (but this then would apply to
>     > ietf-bfd-types, not this document.)
>     > 
>     > 
>     
>     Mahesh Jethanandani
>     [e-mail]
>     
>     
>     
>     
>