Last Call Review of draft-ietf-netmod-geo-location-04
review-ietf-netmod-geo-location-04-yangdoctors-lc-jethanandani-2020-03-23-00

Request Review of draft-ietf-netmod-geo-location
Requested rev. no specific revision (document currently at 07)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2020-03-31
Requested 2020-02-17
Requested by Kent Watsen
Authors Christian Hopps
Draft last updated 2020-03-23
Completed reviews Yangdoctors Last Call review of -04 by Mahesh Jethanandani (diff)
Assignment Reviewer Mahesh Jethanandani 
State Completed
Review review-ietf-netmod-geo-location-04-yangdoctors-lc-jethanandani-2020-03-23
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/GHE3vKrbF8h-l-ioT_fzSa4AKGs
Reviewed rev. 04 (document currently at 07)
Review result Ready with Nits
Review completed: 2020-03-23

Review
review-ietf-netmod-geo-location-04-yangdoctors-lc-jethanandani-2020-03-23

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.

Summary:

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.

Nits

Section 5.1.3

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

Comments:

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 

OLD:
import ietf-yang-types { prefix types; }

NEW:
import ietf-yang-types { 
  prefix types;
  reference
    "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 string??

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
     intentional?


  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.