[netmod] Yangdoctors last call review of draft-ietf-netmod-geo-location-04

Mahesh Jethanandani via Datatracker <noreply@ietf.org> Mon, 23 March 2020 20:08 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: netmod@ietf.org
Delivered-To: netmod@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id BC0003A0E66; Mon, 23 Mar 2020 13:08:56 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: Mahesh Jethanandani via Datatracker <noreply@ietf.org>
To: <yang-doctors@ietf.org>
Cc: netmod@ietf.org, last-call@ietf.org, draft-ietf-netmod-geo-location.all@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.122.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <158499413664.2041.2325993476751382216@ietfa.amsl.com>
Reply-To: Mahesh Jethanandani <mjethanandani@gmail.com>
Date: Mon, 23 Mar 2020 13:08:56 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/W64s6591j05r-IUmOxOE1Xs9hvU>
Subject: [netmod] Yangdoctors last call review of draft-ietf-netmod-geo-location-04
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 23 Mar 2020 20:09:18 -0000

Reviewer: Mahesh Jethanandani
Review result: Ready with Nits

I am not an expert in geo location. If my understanding of the geo location
model is incorrect, feel free to educate me and everyone else. This review is
looking at the draft from a YANG perspective. With that said, I have marked it
as Ready with Nits, because of some of the points discussed below.


This document defines a YANG data model for a generic geographical location. It
is a short document and it is well written and easy to read.


Section 5.1.3

s/mapped using the using the/mapped using the/


Section 3

- Please add references for models that are imported, both in the model and in
the document. For example, in the draft before the YANG model you should have
something like.

This model imports Common YANG Data Types [RFC6991].

And in the model itself

import ietf-yang-types { prefix types; }

import ietf-yang-types {
  prefix types;
    "RFC 6991: Common YANG Data Types.";

The choice of decimal64 with different fractional values must have been
discussed in the WG. However, as the author has noted several times in the
model, that it was chosen over double or strings, even as it left the model
with loss of precision during any conversion to and from e.g. the URI defined
by RFC 5870. It would be worthwhile capturing the reason for the choice, e.g.
the language does not have a built-in type for double, or for not using a

A pyang compilation of the model with —ietf and —lint option was clean. The
example also validated against ietf-geo-location and example YANG model.

A idnits run of the draft reveals a few issues. Cannot say all of them are
valid errors, so ignore them as necessary.

  Checking nits according to https://www.ietf.org/id-info/checklist :

  ** There is 1 instance of too long lines in the document, the longest one
     being 1 character in excess of 72.

  Miscellaneous warnings:

  -- The document date (September 2020) is 176 days in the future.  Is this

  Checking references for intended status: Proposed Standard

     (See RFCs 3967 and 4897 for information about using normative references
     to lower-maturity documents in RFCs)

  -- Possible downref: Non-RFC (?) normative reference: ref. 'EGM08'

  -- Possible downref: Non-RFC (?) normative reference: ref. 'EGM96'

  -- Possible downref: Non-RFC (?) normative reference: ref. 'WGS84'

     Summary: 1 error (**), 0 flaws (~~), 0 warnings (==), 4 comments (--).

     Run idnits with the --verbose option for more detailed information about
     the items above.