Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-yang-05 capitals

Daniele Ceccarelli <daniele.ceccarelli@ericsson.com> Tue, 12 June 2018 15:54 UTC

Return-Path: <daniele.ceccarelli@ericsson.com>
X-Original-To: ccamp@ietfa.amsl.com
Delivered-To: ccamp@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id EFD38130E80 for <ccamp@ietfa.amsl.com>; Tue, 12 Jun 2018 08:54:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.31
X-Spam-Level:
X-Spam-Status: No, score=-4.31 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_DKIMWL_WL_HIGH=-0.01, URIBL_BLOCKED=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=ericsson.com header.b=fAq68af/; dkim=pass (1024-bit key) header.d=ericsson.com header.b=OxfoPyWK
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LbngHqqJGqQU for <ccamp@ietfa.amsl.com>; Tue, 12 Jun 2018 08:54:50 -0700 (PDT)
Received: from sessmg22.ericsson.net (sessmg22.ericsson.net [193.180.251.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 8D29D130E62 for <ccamp@ietf.org>; Tue, 12 Jun 2018 08:54:42 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1528818879; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=an7dWhDAJC2ajvjWs7rO7WPRcKFuaIiad2becX82fCE=; b=fAq68af/ChI2P1+zNuyw9S+I3yPhJHg44X8uG2GZTJO8K/lVAJepWX3/wuXkfRRY k5OFVttS6mkO8TGlNYlmf3NkO31ivgCEzK6gn3N8YIp+4GwEErXELkiW7KTSEGGm 8StHP3E8icrWApV/P9dVaxE9tp1Yad64JWyMmsOzJf4=;
X-AuditID: c1b4fb3a-323229c000005fee-85-5b1fecbfa06b
Received: from ESESSHC002.ericsson.se (Unknown_Domain [153.88.183.24]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id 40.6D.24558.FBCEF1B5; Tue, 12 Jun 2018 17:54:39 +0200 (CEST)
Received: from ESESBMR506.ericsson.se (153.88.183.202) by ESESSHC002.ericsson.se (153.88.183.24) with Microsoft SMTP Server (TLS) id 14.3.382.0; Tue, 12 Jun 2018 17:54:30 +0200
Received: from ESESBMB505.ericsson.se (153.88.183.172) by ESESBMR506.ericsson.se (153.88.183.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Tue, 12 Jun 2018 17:54:30 +0200
Received: from EUR01-VE1-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB505.ericsson.se (153.88.183.172) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Tue, 12 Jun 2018 17:54:30 +0200
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=an7dWhDAJC2ajvjWs7rO7WPRcKFuaIiad2becX82fCE=; b=OxfoPyWK9z4Os02A6tShS19AXyB4B3c8AfjH0h2PZ1P/K1/VWfGdHG1hzao4Fid3d733k3avXBTjDrI848Ke3lgBzvGOeCcvUXJTK8nH06XSvngPrutloEERpzwUz+HBtRaNeAxSWE7NbjUP0YX0EJBqli2OmVt3w/beQBVi7yU=
Received: from HE1PR07MB1675.eurprd07.prod.outlook.com (10.166.124.141) by HE1PR07MB4410.eurprd07.prod.outlook.com (20.176.167.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.863.6; Tue, 12 Jun 2018 15:54:25 +0000
Received: from HE1PR07MB1675.eurprd07.prod.outlook.com ([fe80::6041:9382:35f0:ab5d]) by HE1PR07MB1675.eurprd07.prod.outlook.com ([fe80::6041:9382:35f0:ab5d%3]) with mapi id 15.20.0863.010; Tue, 12 Jun 2018 15:54:24 +0000
From: Daniele Ceccarelli <daniele.ceccarelli@ericsson.com>
To: "t.petch" <ietfc@btconnect.com>, Jonas Ahlberg <jonas.ahlberg@ericsson.com>, Jan Lindblad <janl@tail-f.com>
CC: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "ccamp@ietf.org" <ccamp@ietf.org>, "Yemin (Amy)" <amy.yemin@huawei.com>, "draft-ietf-ccamp-mw-yang.all@ietf.org" <draft-ietf-ccamp-mw-yang.all@ietf.org>, ietf <ietf@ietf.org>
Thread-Topic: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-yang-05 capitals
Thread-Index: AQHUAkTsbtt/9+gds0WlGEtcHPDtJqRcxopw
Date: Tue, 12 Jun 2018 15:54:24 +0000
Message-ID: <HE1PR07MB1675CD879967FBEF488E279AF07F0@HE1PR07MB1675.eurprd07.prod.outlook.com>
References: <152836670897.30871.16818219844116536782@ietfa.amsl.com> <9C5FD3EFA72E1740A3D41BADDE0B461FCF9DE258@dggemm508-mbx.china.huawei.com> <HE1PR0701MB2332F59B6B96EDFC974C823189780@HE1PR0701MB2332.eurprd07.prod.outlook.com> <B986ADA5-4131-43FA-A21B-0C789382EF97@tail-f.com> <HE1PR0701MB2332B8DECF4116EFACA55D9F897F0@HE1PR0701MB2332.eurprd07.prod.outlook.com> <040701d40244$92fa5ce0$4001a8c0@gateway.2wire.net>
In-Reply-To: <040701d40244$92fa5ce0$4001a8c0@gateway.2wire.net>
Accept-Language: it-IT, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is ) smtp.mailfrom=daniele.ceccarelli@ericsson.com;
x-originating-ip: [198.24.6.136]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; HE1PR07MB4410; 7:eZUQ40zrVsPAiqB4RsAJpJTHgEhhitynA1YSoCWaeqyMV7PpcGWXWOHacp50HOkkRcJ4B3Tx8j0A0Q+y0nHHqIXBXX4RMoOcFAr78ct1+xA/RUwFMpY3Ei4v6GvjM2J/hXpG2HvjDd08Lrnd+JcQa12Y+UqNA6HxFAj0VP0+opFWbqfBq/bFbziy1XMKCaRKjv/8vq7KJ7fXDBSaSD6dlhPLb4dsuLtPkepc1imrJM5PltLHhIE26u/YCQoOCfNq
x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR;
x-forefront-antispam-report: SFV:SKI; SCL:-1; SFV:NSPM; SFS:(10009020)(346002)(39380400002)(39860400002)(366004)(396003)(376002)(13464003)(76094002)(40764003)(199004)(189003)(51444003)(99286004)(53546011)(102836004)(44832011)(53946003)(14454004)(55016002)(345774005)(2906002)(76176011)(26005)(59450400001)(8666007)(186003)(3660700001)(97736004)(6116002)(6436002)(6506007)(9686003)(3280700002)(86362001)(3846002)(105586002)(4326008)(7736002)(81166006)(305945005)(74316002)(478600001)(81156014)(8676002)(5250100002)(33656002)(66066001)(5660300001)(53936002)(7696005)(446003)(2900100001)(11346002)(296002)(316002)(93886005)(6246003)(229853002)(106356001)(25786009)(486006)(68736007)(476003)(561944003)(110136005)(551934003)(8936002)(54906003)(579004)(559001); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR07MB4410; H:HE1PR07MB1675.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1;
x-microsoft-antispam: UriScan:(219612443155931); BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:HE1PR07MB4410;
x-ms-traffictypediagnostic: HE1PR07MB4410:
x-microsoft-antispam-prvs: <HE1PR07MB441007772C769C5EE0BB2151F07F0@HE1PR07MB4410.eurprd07.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(37575265505322)(178726229863574)(278428928389397)(50582790962513)(219612443155931)(788757137089)(17755550239193);
x-ms-exchange-senderadcheck: 1
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3231254)(944501410)(52105095)(10201501046)(93006095)(93001095)(3002001)(149027)(150027)(6041310)(20161123562045)(20161123564045)(201703131423095)(201703031522075)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123560045)(6072148)(201708071742011)(7699016); SRVR:HE1PR07MB4410; BCL:0; PCL:0; RULEID:; SRVR:HE1PR07MB4410;
x-forefront-prvs: 07013D7479
received-spf: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts)
x-microsoft-antispam-message-info: oYX+u85IP2KEUVuJg659EYV0wMHuA4MGrtGduMx34qdafN/0Ro1ebTZAmyF07OxLz96nf2oTr8W8WGd9wFCmWjuKWyr/fWdUr/psqwS4zHBzGXdRk7uwsL3spPB0w+H0TIYzA1VmFQUBmT/0GjghCN3k8t9JSt4aAd2EzyLdmwfqn3v0Dpb/CPvH8IkcimGs
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-MS-Office365-Filtering-Correlation-Id: 442c1614-cb5e-4b09-0936-08d5d07cc55b
X-MS-Exchange-CrossTenant-Network-Message-Id: 442c1614-cb5e-4b09-0936-08d5d07cc55b
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jun 2018 15:54:24.7081 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 92e84ceb-fbfd-47ab-be52-080c6b87953f
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR07MB4410
X-OriginatorOrg: ericsson.com
X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTYRjG+3a+bcfl8rQUXwxRFwkZaYp/HCRMk8AixC6QzChXHi+p03ZM 0iBspuYFEa+58kJbeIk0dZKZZN4QhSzS1NSwdLbUVEyQnLfmzoL++z3f87zf+74fH0lI6vkO ZLQikVEq5LFSgQiXhbyCYx2/nEKPV309Sjc/fCmg9U/GMD041MyjfzRWYnpkegrTO+o8ROe1 vUN+wsC23j/CwAc9i/xArXadF9ho1OBgLBOdCGdio5MYpYdvmCjqy8csnDBSx7tTvNGOU1Gq hpeNrEigvGFrtlaYjUSkhOpB0FuzjDihQ1CxMmxx1hAYMyYsjpYHj/SrZoGpfAImdgox5xTy YCZTY6mZRpBZrjMJkhRQPqDvOrfb0ZaKh6XNRv4uE9QcghX95V0+QIXC41GdgMtcgQVdGcGx F+xUZ6FdxtRhGCuaNk8uNmXe980IuF7VBKh+q8zFVtRJSB+cMDOiHCH/zVPENbOHcX2lZW0K tO0fCI7tYG5mm8/lr0NDeqsl4wyG2RnMsSN8qswxrwyUjgfLGXoBZ3hCX20HwRlrAhjNGMCc 6EOwWDxkucoNVCOTfI5joGXrpyX0GMF2SbcgH3mp/xtRbXoygjoCDW0e3LELFOV8F6rNa++H /jI9rkK4DtmxDMvGRXp5uTPK6BssG69wVzCJTcj0izp1Gz6tqNPg34UoEkmtxWGfnUIlfHkS mxzXhYAkpLbi8y2mI3G4PDmFUcZfU96OZdgudJDEUnvxqQhaJqEi5YlMDMMkMMp/Lo+0ckhF VofSLjm75M4HhBr2uNTU958xivbeH4hggjwKNAmbNONsqBhfsnlxWhoU9m1fcEh8aUrVakfI WMmwon6+rGBWFTB5r2K9vH/0WURUU/pNm2j1Wadyma2Lf7ZnzYK13duLMaWyWw6+r12ft1zw roCrGpWfsW1Kkgt3XdMya7tZVylmo+SeboSSlf8FHRKaz0EDAAA=
Archived-At: <https://mailarchive.ietf.org/arch/msg/ccamp/Qw-z0i8jTlMnDa4FXdYns3d9Qpk>
Subject: Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-yang-05 capitals
X-BeenThere: ccamp@ietf.org
X-Mailman-Version: 2.1.26
Precedence: list
List-Id: Discussion list for the CCAMP working group <ccamp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ccamp>, <mailto:ccamp-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ccamp/>
List-Post: <mailto:ccamp@ietf.org>
List-Help: <mailto:ccamp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ccamp>, <mailto:ccamp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 12 Jun 2018 15:54:53 -0000

Agree with Tom here, if it is not a big issue I'd prefer to keep them in capital letter as they show up in capital letters in all the draft/RFC and IANA registries.

BR
Daniele  

> -----Original Message-----
> From: t.petch <ietfc@btconnect.com>
> Sent: martedì 12 giugno 2018 06:56
> To: Jonas Ahlberg <jonas.ahlberg@ericsson.com>; Jan Lindblad <janl@tail-
> f.com>
> Cc: yang-doctors@ietf.org; ccamp@ietf.org; Yemin (Amy)
> <amy.yemin@huawei.com>; draft-ietf-ccamp-mw-yang.all@ietf.org; ietf
> <ietf@ietf.org>
> Subject: Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-
> yang-05 capitals
> 
> Top posting and tweaking the subject line slightly since I am picking up one
> minor point only
> 
> > There are a couple of identities with capitals. Consider changing to
> all lowercase; that is the YANG convention.
> >
> >   identity E1 {
> >   identity STM-1 {
> > [Amy] will fix it.
> > [Jonas] OK.
> 
> Disagree!  If I see e1 I will probably think exponent - if I see E1, I know exactly
> what is intended, in a wide range of contexts.  YANG guidelines allows
> "Upper-case characters, the period character,
>    and the underscore character MAY be used if the identifier represents
>    a well-known value that uses these characters.  "
> and I think that E1 and STM-1 fall within that MAY.
> 
> I see
> identity client-signal-OC3_STM1
> in
> draft-ietf-ccamp-otn-tunnel-model-01
> and
> identity STM-1 {
> in
> draft-fioccola-ccamp-l1csm-yang-01
> 
> so I think that CCAMP needs to decide between STM-1 and STM1 - the
> former works for me - and then tell the YANG doctors that that is how it
> is:-) YMMV
> 
> Tom Petch
> 
> ----- Original Message -----
> From: "Jonas Ahlberg" <jonas.ahlberg@ericsson.com>
> To: "Jan Lindblad" <janl@tail-f.com>
> 
> 
> > Hi Jan & Amy,
> > See comments & questions in green [Jonas2].
> > Regards
> > JonasA
> >
> > From: Jan Lindblad <janl@tail-f.com>
> > Sent: den 11 juni 2018 15:29
> 
> >
> > Jonas,
> >
> > Thank you for your review and valuable comments.
> > I’ve not responded to the latest mail since I have added comments to
> some of the items removed in that mail.
> > I’ve copied and pasted your comments into this mail instead, in order
> to make the thread of comments complete.
> >
> > I’ve added my comments in green [Jonas].
> >
> > [janl2] Very good. Sorry for deleting text prematurely ;-)
> >
> > I would like to ask CCAMP for support with the question about if the
> protection group is likely to be reused elsewhere or if it can be merged with
> the microwave module again.
> >
> > Regards
> > JonasA
> >
> > From: Yemin (Amy)
> <amy.yemin@huawei.com<mailto:amy.yemin@huawei.com>>
> > Sent: den 8 juni 2018 11:37
> >
> > Hi Jan,
> >
> > Thanks very much for the comments. I found them are quiet useful to
> improve the models.
> > Please see my reply below in blue.
> > The draft co-authors may add more reply to your comments.
> >
> > BR,
> > Amy
> > -----Original Message-----
> > From: CCAMP [mailto:ccamp-bounces@ietf.org] On Behalf Of Jan Lindblad
> > Sent: Thursday, June 07, 2018 6:18 PM
> >
> > Reviewer: Jan Lindblad
> > Review result: Ready with Issues
> >
> > YD review of draft-ietf-ccamp-mw-yang
> >
> > I have now reviewed the YANG modules and corresponding examples of
> the -05 version of this draft. I have not read much of the RFC text, so I can't
> vouch for how well the text aligns with the model. I find the proposed
> modules in good shape. Most of my comments below are alternate ways of
> modeling something that the wg may consider, or a few things where I
> propose a better option for something that would be acceptable even as it is.
> There is a single issue that must be fixed IMO, see #8 below.
> >
> > Let's start with module ietf-microwave-types.
> >
> > #1) Consider adding structure to related identities
> >
> > This module consists mostly of a long list of identities based off of
> coding-modulation. If it makes sense that in the future someone might be
> interested in doing something with all qam-4096 based identities or all -
> strong identities, say, it may make sense to model the identities as based on
> each other in a tree style. E.g.
> >
> >   identity qam-4096 {
> >     base coding-modulation;
> >     description
> >       "4096 QAM coding and modulation scheme.";
> >   }
> >
> >   identity qam-4096-strong {
> >     base qam-4096;
> >     description
> >       "4096 QAM strong coding and modulation scheme.";
> >   }
> >
> >   identity qam-4096-light {
> >     base qam-4096;
> >     description
> >       "4096 QAM light coding and modulation scheme.";
> >   }
> >
> > Or even go to "multiple inheritance" with multiple bases for
> identities, e.g.
> > for qam-4096 and strong. This would allow future applications to
> filter the identities on such criteria. Just a thought.
> > [Amy] change to base qam-4096. Multiple bases seem not applicable
> here.
> > [Jonas] I agree.
> >
> > #2) Convention to use all lowercase in YANG symbols
> >
> > There are a couple of identities with capitals. Consider changing to
> all lowercase; that is the YANG convention.
> >
> >   identity E1 {
> >   identity STM-1 {
> > [Amy] will fix it.
> > [Jonas] OK.
> >
> > Next, let's look at module ietf-interface-protection.
> >
> > I can't say I understand exactly why this is a separate module. It
> publishes a single grouping, which is required by ietf-microwave-radio-link,
> and as far as I understand would probably never be used anywhere else.
> When the grouping is used a single time in ietf-microwave-radio-link, it is
> immediately refined.
> > Would probably reduce the clutter by merging the two modules and
> resolving the refine.
> > [Amy] They were in one model. During the WG discussion, comment was
> raised that the interface protection function could be generic and be used by
> other technologies in future, so we split the models.
> > I’m open to discuss about this.
> > [janl] Ok, I didn't quite see how the grouping would be used anywhere
> else, but if it indeed is usable elsewhere, having it in a separate module isn't
> a bad idea.
> > [Jonas] I’m in favor of reverting back to one single module gain,
> since it will simplify the model as you mention above. The argument for
> breaking it out into a separate module is that it can be used elsewhere.
> I’m doubtful if it ever will be, but here I think we need input from the rest of
> the community (at least from the CCAMP WG).
> >
> >
> > #3) Config true leaf name status
> >
> > I find it counter-intuitive that a leaf called status (or state) is a
> configuration item. I had to re-read the model several times to get my head
> around the fact that this is indeed meant to be config true.
> Perhaps consider a name change?
> >
> >       leaf status {
> > [Amy] status should be config false.
> > [Jonas] Correct. Our mistake.
> >
> > #4) Action external-commands
> >
> > There is a single action called external-commands (even in plural). It
> takes a single argument, which is the name of the operation to execute.
> No output. To me, a more natural modeling would be to make each of the
> external commands an action, over time possibly with different input and
> output.
> > [Amy] add output to describe the action result (success, fail,
> inprogress). But prefer to use one action.
> > Change the name to external-command.
> > [janl] Hmm, I don't quite understand your preference for a single
> action. I see how this choice closes doors, but no real benefit. This is no big
> deal, though, what you have works. Just feels less evolvable for no reason.
> > [Jonas] I’m not quite sure I’ve understood what you propose. Do you
> mean that we should replace the single action statement and the list of
> identifiers with several action statements:
> > action manual-switch-working;
> > action manual-switch-protection;
> > …
> > action clear;
> > The reason for using identities was that we wanted to make it easy to
> extend the module with additional, potentially vendor specific, commands by
> augmentation.
> > But maybe there are other ways to handle that in case of using the
> alternative approach you suggest.
> >
> > [janl2] Yes, exactly. Separate actions like this has the advantage
> that they can take (different) input and output parameters, and that they
> can be access controlled separately using NACM. This way different
> operators could have/not have access to different operations. Future
> standard or vendor modules could still augment whatever additional actions
> they like to the same location in the tree.
> > [Jonas2] OK I understand and I see the advantage of this approach.
> Amy, should we consider change to this or are there other benefits with the
> original approach?
> >
> > Finally, we have module ietf-microwave-radio-link.
> >
> > #5) Use derived-from when comparing identities
> >
> > It's more future-proof and more likely to be interoperable if you use
> proper XPATH functions to determine kinship than using plain equality
> >
> >   augment "/if:interfaces/if:interface" {
> >     when "if:type = 'mw-types:radio-link-terminal'";
> >
> > is better written as
> >
> >   augment "/if:interfaces/if:interface" {
> >     when "derived-from-or-self(if:type,
> 'mw-types:radio-link-terminal')";
> >
> > This allows future sub-typing (sub-classing) of radio-link-terminal,
> i.e. new identities that are based on radio-link-terminal to reflect some
> special kind of RLT. It also improves chances of interoperability.
> > [Amy] will adopt this.
> > [Jonas] Agree.
> >
> > #6) Blank id reasonable?
> >
> > Leafs that function as an id usually do not have defaults. Does a
> blank id make sense here? If it does, maybe it would make more sense to
> leave it without a default and explain what happens if not set in the
> description instead? Or mark it as mandatory if it has to be set.
> >
> >     leaf id {
> >       type string;
> >       default "";
> > [Amy] remove default. Add mandatory.
> > [Jonas] I think that the name of the attribute could be misleading. It
> ’s not a formal id of the terminal, it is a text string that is used by an optional
> system feature in a far-end RLT (on the opposite end of the microwave link)
> to verify that the correct two RLTs (far-end and
> near-end) are connected.
> > I don’t think we should make it mandatory. I would suggest to leave it
> without default and clarify the consequences in the description.
> >
> > #7) Use derived-from when comparing identities (again)
> >
> >     leaf-list carrier-terminations {
> >       type if:interface-ref;
> >       must "/if:interfaces/if:interface[if:name = current()]"
> >          + "/if:type = 'mw-types:carrier-termination'" {
> >
> > is better written as
> >
> >       must "derived-from-or-self(/if:interfaces/if:interface[if:name =
> >       current()]"
> >          + "/if:type, 'mw-types:carrier-termination')" {
> >
> > It is possible to write this in a more compact way, but there's
> nothing wrong with the above.
> >
> >       must "derived-from-or-self(deref(current())/.."
> >          + "/if:type, 'mw-types:carrier-termination')" { [Amy] will
> > adopt this.
> > [Jonas] Agree.
> >
> > #8) Badly broken frequency duplex config
> >
> > If you read the descriptions in these related leafs:
> >
> >     leaf tx-frequency {
> >       type uint32;
> >       units "kHz";
> >       mandatory true;
> >       description
> >         "Selected transmitter frequency.";
> >     }
> >    leaf rx-frequency {
> >       type uint32;
> >       units "kHz";
> >       description
> >         "Selected receiver frequency.
> >          Overrides existing value in duplex-distance.
> >          Calculated from tx-frequency and duplex-distance if
> >          only duplex-distance is configured.
> >          Must match duplex-distance if both leaves are
> >          configured in a single operation.";
> >     }
> >
> >     leaf duplex-distance {
> >       type uint32;
> >       units "kHz";
> >       description
> >         "Distance between Tx & Rx frequencies.
> >          Used to calculate rx-frequency when
> >          rx-frequency is not specifically configured.
> >          Overrides existing value in rx-frequency.
> >          Calculated from tx-frequency and rx-frequency if only
> >          rx-frequency is configured.
> >          Must match rx-frequency if both leaves are configured
> >          in a single operation.";
> >     }
> >
> > It appears that the author intends the system to fill in the value for
> one of these leaves based on the value set for the other. This is a big no-no.
> A system should never alter its own configuration, or automation flows
> (which is the whole purpose with YANG and NETCONF, remember) will
> break. Also, the validity of the configuration should not depend on how
> many operations are used to inject it.
> >
> > I find this a serious flaw that must be fixed before the module can be
> released.
> >
> > I propose fixing it like this:
> >
> >     leaf tx-frequency {
> >       type uint32;
> >       units "kHz";
> >       mandatory true;
> >       description
> >         "Selected transmitter frequency.";
> >     }
> >     choice freq-or-distance {
> >       leaf rx-frequency {
> >         type uint32;
> >         units "kHz";
> >         description
> >           "Selected receiver frequency."
> >       }
> >       leaf duplex-distance {
> >         type uint32;
> >         units "kHz";
> >         description
> >           "Distance between Tx & Rx frequencies."
> >       }
> >     }
> >
> > If you would like to have read-only computed values accessible in the
> model, maybe you could add:
> >
> >     leaf actual-rx-frequency {
> >       type uint32;
> >       units "kHz";
> >       description
> >         "Computed receiver frequency."
> >       config false;
> >     }
> >     leaf actual-duplex-distance {
> >       type uint32;
> >       units "kHz";
> >       description
> >         "Computed distance between Tx & Rx frequencies."
> >       config false;
> >     }
> >
> > Many other ways of doing this properly are also possible. Let me know
> if you'd like to discuss options.
> > [Amy] I think choice is a good way to model those leafs. I suggest to
> use it.
> > That’s the real value of YANG doctors! Thanks.
> > [Jonas] Thanks for your help. We have discussed this a lot without
> finding a good solution. I agree that we should use it.
> > The only question I have is the name of the attributes. Should the
> attribute used for configuration or the attribute showing the resulting be
> called rx-frequency?
> > config-rx-frequency & rx-frequency   v.s.   rx-frequency &
> actual-rx-frequency
> >
> > [janl2] I used actual-rx-frequency because you had some other leafs
> called actual-...
> >
> > Yet another possibility could be to model it like this (pseudo YANG):
> >
> > leaf tx { ... }
> > choice freq-or-dist {
> >   container by-frequency {
> >     leaf rx { ... }
> >     leaf dist { config false; }
> >   }
> >   container by-distance {
> >     leaf dist { ... }
> >     leaf rx { config false; }
> >   }
> > }
> >
> > I.e. three leaves with the same names, just different paths (since the
> container names differ) and different configness. I think my initial proposal is
> a little cleaner, tho, which is why I went with that one earlier. Or you could
> move the actual-... leaves into the choice, so that only the one that's
> meaningful exists.
> >
> > [Jonas2] I think that your first proposal has a benefit of always
> providing access to the actual configuration through the same leafs (and
> path) (tx-frequency, actual-rx-frequency, actual-duplex-distance,
> independent of how they have been configured.
> > One use-case we have discussed is that an operator uses
> duplex-distance for the initial configuration of the rx-frequency and then in a
> second step fine tuning it by setting the rx-frequency directly. Would that
> work with a choice statement, i.e. that you first choose duplex-distance and
> then in a subsequent operation overrides that and choose rx-frequency?
> >
> > #9) Check that lower threshold is less than upper threshold
> >
> > Would it make sense to add a must statement checking that the lower
> threshold is less than (or equal?) to the upper threshold?
> >
> >     leaf atpc-lower-threshold  {
> >       when "../power-mode = 'atpc'";
> >       type power {
> >         range "-99..-30";
> >       }
> >       units "dBm";
> >       mandatory true;
> >       description
> >         "The lower threshold for the input power at far-end
> >          used in the ATPC mode.";
> >       reference "ETSI EN 302 217-1";
> >     }
> >     leaf atpc-upper-threshold  {
> > [Amy] can add the must statement.
> > [Jonas] Good!
> >
> > #10) Choice more convenient
> >
> > There are a few leafs that act as discriminators for when clauses in
> other leafs. Such constructs might be a little smoother when modeled as a
> choice instead. I'll take one and show as an example. This power-mode
> construct:
> >
> >     leaf power-mode {
> >       type enumeration {
> >         enum rtpc {
> >           description
> >             "Remote Transmit Power Control (RTPC).";
> >           reference "ETSI EN 302 217-1";
> >         }
> >
> >         enum atpc {
> >           description
> >             "Automatic Transmit Power Control (ATPC).";
> >           reference "ETSI EN 302 217-1";
> >         }
> >       }
> >       mandatory true;
> >       description
> >         "A choice of Remote Transmit Power Control (RTPC)
> >          or Automatic Transmit Power Control (ATPC).";
> >     }
> >
> >     leaf maximum-nominal-power {
> >       type power {
> >         range "-99..40";
> >       }
> >       units "dBm";
> >       mandatory true;
> >       description
> >         "Selected output power in RTPC mode and selected
> >          maximum output power in ATPC mode. Minimum output
> >          power in ATPC mode is the same as the system
> >          capability, available-min-output-power.";
> >       reference "ETSI EN 302 217-1";
> >     }
> >
> >     leaf atpc-lower-threshold  {
> >       when "../power-mode = 'atpc'";
> >       type power {
> >         range "-99..-30";
> >       }
> >       units "dBm";
> >       mandatory true;
> >       description
> >         "The lower threshold for the input power at far-end
> >          used in the ATPC mode.";
> >       reference "ETSI EN 302 217-1";
> >     }
> >     leaf atpc-upper-threshold  {
> >       when "../power-mode = 'atpc'";
> >       type power {
> >         range "-99..-30";
> >       }
> >       units "dBm";
> >       mandatory true;
> >       description
> >         "The upper threshold for the input power at far-end
> >          used in the ATPC mode.";
> >       reference "ETSI EN 302 217-1";
> >     }
> >
> > could be modeled as:
> >
> >     choice power-mode {
> >       container rtpc {
> >         description
> >           "Remote Transmit Power Control (RTPC).";
> >         reference "ETSI EN 302 217-1";
> >         leaf maximum-nominal-power {
> >           type power {
> >             range "-99..40";
> >           }
> >           units "dBm";
> >           mandatory true;
> >           description
> >             "Selected output power.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >       }
> >       container atpc {
> >         description
> >           "Automatic Transmit Power Control (ATPC).";
> >         reference "ETSI EN 302 217-1";
> >
> >         leaf maximum-nominal-power {
> >           type power {
> >             range "-99..40";
> >           }
> >           units "dBm";
> >           mandatory true;
> >           description
> >              "Selected maximum output power. Minimum output
> >              power is the same as the system
> >              capability, available-min-output-power.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >
> >         leaf atpc-lower-threshold  {
> >           type power {
> >             range "-99..-30";
> >           }
> >           units "dBm";
> >           mandatory true;
> >           description
> >             "The lower threshold for the input power at far-end
> >              used in the ATPC mode.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >         leaf atpc-upper-threshold  {
> >           type power {
> >             range "-99..-30";
> >           }
> >           units "dBm";
> >           mandatory true;
> >           description
> >             "The upper threshold for the input power at far-end
> >              used in the ATPC mode.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >       }
> >       mandatory true;
> >       description
> >         "A choice of Remote Transmit Power Control (RTPC)
> >          or Automatic Transmit Power Control (ATPC)."; }
> >
> > [Amy] Choice is a better way. Is it possible to further refine your
> proposal?
> > Since maximum-nominal-power will be used by both RTPC and ATPC, how
> about to move it out of the choice, then use maximum-nominal-power in the
> choice? Like this:
> >         leaf maximum-nominal-power {
> >           type power {
> >             range "-99..40";
> >           }
> >           units "dBm";
> >          mandatory true;
> >           description
> >              "Selected maximum output power. Minimum output
> >              power is the same as the system
> >              capability, available-min-output-power.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >
> >     choice power-mode {
> >       container rtpc {
> >         description
> >           "Remote Transmit Power Control (RTPC).";
> >         reference "ETSI EN 302 217-1";
> >
> >         use maximum-nominal-power;
> >       }
> >       container atpc {
> >         description
> >           "Automatic Transmit Power Control (ATPC).";
> >         reference "ETSI EN 302 217-1";
> >
> >         use maximum-nominal-power;
> >
> >         leaf atpc-lower-threshold  {
> >           type power {
> >             range "-99..-30";
> >           }
> >           units "dBm";
> >           mandatory true;
> >           description
> >             "The lower threshold for the input power at far-end
> >              used in the ATPC mode.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >         leaf atpc-upper-threshold  {
> >           type power {
> >             range "-99..-30";
> >           }
> >           units "dBm";
> >           mandatory true;
> >           description
> >             "The upper threshold for the input power at far-end
> >              used in the ATPC mode.";
> >           reference "ETSI EN 302 217-1";
> >         }
> >       }
> >       mandatory true;
> >       description
> >         "A choice of Remote Transmit Power Control (RTPC)
> >          or Automatic Transmit Power Control (ATPC).";
> >     }
> >
> > [janl] Yes, this is certainly very reasonable. I placed the
> maximum-nominal-power inside the choice because the description string
> was giving the leaf different interpretations in each case. In that case I felt it
> better to have separate leaves with clear descriptions. If you feel the leaf can
> be shared between the options with a clear definition of its meaning, this is
> reasonable.
> > [Jonas] One purpose of the mode attribute was to show if the
> carrier-termination was configured for RTPC or ATPC, but I guess the same
> can be achieved by looking for which container, rtpc or atpc, has been
> instantiated.
> >
> > [janl2] Yes, that's the idea.
> > [Jonas2]. Good!
> >
> > I suggest that we go for Jan’s proposal since the attribute has
> different meaning in the two cases. One purpose of the
> >
> > #11) Choice more convenient (again)
> >
> > Same thing again for
> >
> >     leaf coding-modulation-mode {
> > [Amy] will update it.
> > [Jonas] OK, see above.
> >
> > #12) Unusual exponential notation
> >
> > Do you really mean 10e-9 (=10*10^-9 =10^-8), or do you mean the more
> traditional notation 1e-9?
> >
> >       leaf ber-alarm-threshold {
> >         type enumeration {
> >           enum "10e-9" {
> > [Amy] should be 1e-9. Need to confirm with the co-authors.
> > [Jonas] Yes, we mean 10^-9, 10^-8, …, 10^-1.
> >
> > [janl2] A perpahs simpler way to model this would be to only keep the
> part that contains information, i.e. as
> >
> > leaf ber-alarm-threshold {
> >   type int8 {
> >     range -9..-1;
> >   }
> >   default -6;
> >   description "Specification of at which BER as a power of 10 an alarm
> should be raised. 10^-6 is the default.";
> >   reference "ETSI EN 302 217-1";
> > }
> >
> > #13) Separate module with grouping, used a single time with refine
> >
> > Module ietf-interface-protection defines a grouping
> (protection-groups), which is used a single time, yet is refined when it is used
> below. As noted before comment #3, I find this way of laying out the YANG
> unnecessarily hard to read and understand, for no clear benefit.
> >
> >   container radio-link-protection-groups { ....
> >     uses ifprot:protection-groups {
> >
> >       refine protection-group/members {
> >         must "/if:interfaces/if:interface[if:name = current()]"
> >            + "/if:type = 'mw-types:carrier-termination'" {
> >
> > Also, as noted in comment #7, the must statement is better written
> using derived-from-or-self. This applies regardless of the current refine
> statement is kept, or if the must statement moves to the actual leaf-list it
> applies to.
> > [Amy] Will update the must statement.
> > [Jonas] If I remember correctly, I think it was done with the ambition
> that it should be made generic so that other interface technologies could use
> the same protection module. In case, we revert back to one single module I
> suggest that we also simply the structure as Jan suggests.
> >
> > Appendix A.1 & A.2
> >
> > Besides the actual YANG modules, there are also a couple of examples
> in Appendix A.1 and A.2. I tried to use them and uncovered a couple of
> issues.
> >
> > #14) Config false leaf in config example
> >
> > Both examples list
> >
> >         "tx-oper-status": on
> >
> > This is a config false item, which could never be part of a
> configuration message (and it also lacks comma at the end).
> > [Amy] will remove it.
> > [Jonas] OK
> >
> > #15) Wrong type in example
> >
> > Both examples list
> >
> >         "coding-modulation-mode": 0,
> >
> > 0 not a legal value, should be "single" to match the rest of the
> example data.
> > [Amy] will fix it.
> > [Jonas] OK
> >
> > #16) Missing mandatory parameter
> >
> > Both examples lacks leaf maximum-nominal-power, which is mandatory
> according to the YANG, so the transaction fails to validate.
> > [Amy] will add it.
> > [Jonas] OK
> >
> > #17) Examples perhaps a tad basic
> >
> > The examples are demonstrating only a small part of the module
> functionality. A bigger example, e.g. including xpic with interface pointers
> might be useful.
> >
> > Feel free to reach out to me to discuss any of this. Thank you.
> > [Amy] will add additional example.
> > [Jonas] Good suggestion. Amy, let’s what configuration to show.
> >
> >
> > Best Regards,
> > /jan
> >
> >