Hi Chris, Thank you and the WG for your work on this document. I've requested IETF LC.
Regards, Rob From: Christian Hopps <[email protected]> Sent: 16 April 2021 22:30 To: Rob Wilton (rwilton) <[email protected]> Cc: [email protected]; [email protected] Subject: Re: AD review of draft-ietf-netmod-geo-location-07 Just posted this new version. On Apr 13, 2021, at 6:08 AM, Rob Wilton (rwilton) <[email protected]<mailto:[email protected]>> wrote: Hi Chris, -----Original Message----- From: Christian Hopps <[email protected]<mailto:[email protected]>> Sent: 13 April 2021 03:38 To: Rob Wilton (rwilton) <[email protected]<mailto:[email protected]>> Cc: Christian Hopps <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]>; draft-ietf- [email protected]<mailto:[email protected]> Subject: Re: AD review of draft-ietf-netmod-geo-location-07 On Mar 7, 2021, at 3:48 PM, Rob Wilton (rwilton) <[email protected]<mailto:[email protected]>> wrote: Hi Chris, Apologies for the delayed AD review of this document. I found that this document to be interesting, enlightening, and well written, so thank you. I only have a few minor comments, the rest are grammatical nits. Some I spotted manually; the rest are from a beta version of a grammar tool that I am playing with. Minor comments/questions: 1. In the YANG module, the pattern statements have 3 ranges of characters, but the description only indicates two ranges. Is there a reason that the following pattern doesn't work? pattern '[ -@\[-~]*'; >From an old email on the list: "These 3 ranges seem to make pyang happy. I don't know why I need to break up the second range into 2 adjacent ranges like that to make pyang not complain, but complain it does if I just use: '[ -@\[-~]*'" [RW] Okay. 2. I note that the YANG module allows just a lat, or a long, or a height to be specified, rather than requiring at least a pair of lat/long or x/y coordinates. I think that this is fine (and keeps the model flexible), but wanted to check that this is the intentional. 3. I also note that that grouping doesn't require that any coordinates be specified at all. I presume that this is intentional, it makes sense to me (e.g., if it is intended to be optional). My answer to a thread on the list asking for removal of some of the mandatory designations which cause problems: "The intention was for a location node to be required if the <geo- location> container was present. Apparently this will only work if "container geo-lcoation" is a presence container. I'm not even sure that's all that smart of a requirement (e.g., maybe someone just wants to indicate the reference-frame for an object). I'll remove the mandatory from location." [RW] Okay. 4. In the YANG module, "v-east is the rate of change (i.e., speed) perpendicular to truth-north as defined by the geodetic-system."; As a nit, this doesn't actually define whether a positive v-east value is in the East or West direction. I appreciate that this is obvious, but for the other two components in the vector, it is unambiguously specified. How about: "v-east is the rate of change (i.e., speed) perpendicular to the right of truth-north as defined by the geodetic-system."; [RW] LGTM. 5. In Security Considerations: Since the grouping defined in this module identifies locations, authors using this grouping SHOULD consider any privacy issues that may arise when the data is readable. Perhaps, expand this paragraph to give an example, e.g., revealing the physical location of a device, or data center. How about: "Since the grouping defined in this module identifies locations, authors using this grouping SHOULD consider any privacy issues that may arise when the data is readable (e.g., customer device locations, etc)." [RW] LGTM. The rest are just grammar nits: In addition to identifying the astronomical body we also need to define the meaning of the coordinates => In addition to identifying the astronomical body, we also need to define the meaning of the coordinates fixed In addition to the "geodetic-datum" value we allow refining the coordinate and height accuracy using "coord-accuracy" and "height- accuracy" respectively. => In addition to the "geodetic-datum" value, we allow refinement of the coordinate and height accuracy using "coord-accuracy" and "height- accuracy" respectively. fixed This is the location on or relative to the astronomical object. It is specified using 2 or 3 coordinates values. => This is the location on, or relative to, the astronomical object. It is specified using 2 or 3 coordinates values. fixed The intent of the grouping being defined here is to identify where something is located, and generally this is expected to be somewhere on or relative to Earth (or another astronomical body). => The intent of the grouping being defined here is to identify where something is located, and generally this is expected to be somewhere on, or relative to, Earth (or another astronomical body). fixed At least two options are available to YANG models that wish to use this grouping with objects that are changing location frequently in non- simple ways, they can add additional motion data to their model directly, or if the application allows it can require more frequent queries to keep the location data current. => At least two options are available to YANG models that wish to use this grouping with objects that are changing location frequently in non- simple ways. They can add additional motion data to their model directly. Or, if the application allows, it can require more frequent queries to keep the location data current. fixed When coord-accuracy is specified it overrides the geodetic-datum implied accuracy. => When coord-accuracy is specified, it overrides the geodetic-datum implied accuracy. fixed When specified it overrides the geodetic-datum implied default. => When specified, it overrides the geodetic-datum implied default. fixed indicated by the reference-frame value. => indicated by the reference-frame. fixed For a formula to convert these values to speed and heading see this modules defining document RFC XXXX."; => For a formula to convert these values to speed and heading see RFC XXXX."; fixed You have "truth-north" and "truth north" and "true-north". Should these all be "true north"? fixed YANG grouping using decimal64 values rather than strings. For the relative height cases the application doing the transformation is expected to have the data available to transform the relative height into an absolute height which can then be expressed using the YANG grouping. => YANG grouping using decimal64 values rather than strings. For the relative height cases, the application doing the transformation is expected to have the data available to transform the relative height into an absolute height, which can then be expressed using the YANG grouping. fixed Grammar Warnings (generated by a tool): Draft Text: Indeed it is easy to imagine a network or device located on The Moon, on Mars, on Enceladus (the moon of Saturn) or even a comet (e.g., 67p/churyumov-gerasimenko). Warning: Did you forget a comma after a conjunctive/linking adverb? Suggested change: "Indeed," fixed Draft Text: This document defines a "geo-location" YANG grouping that allows for all of the above data to be captured. Warning: Consider using all the. Suggested change: "all the" fixed Draft Text: When specified these values override the defaults implied by the "geodetic-datum" value. Warning: "When" at the beginning of a sentence usually requires a 2nd clause. Maybe a comma, question or exclamation mark is missing, or the sentence is incomplete and should be joined with the following sentence. Suggested change: "When specified, " fixed Draft Text: In both choices the exact meanings of all of the values are defined by the "geodetic-datum" value in the [xref]. Warning: Consider using all the. Suggested change: "all the" fixed Draft Text: During the development of this module, the question of whether it would support data such as orientation arose. Warning: Wordiness: Consider shortening this phrase. Suggested change: "whether" ? [RW] Ignore. Draft Text: For test "A.1.2.1" the YANG geo location object either includes a CRS ("reference-frame") or has a default defined ([xref]). Warning: This word is normally spelled as one. Suggested change: "geolocation" unchanged. Draft Text: Many systems make use of geo-location data, and so it's important to be able describe this data using this geo-location object defined in this document. Warning: The preposition 'to' is required in front of the verb 'describe'. Suggested change: "able to describe" Removed this paragraph it's unneeded and not very clear. Draft Text: For accuracy it has a single "u" parameter for specifying uncertainty. Warning: The comma is probably missing here: accuracy, it. Suggested change: "accuracy, it" fixed Draft Text: This is used by many application (e.g., Google Maps API). Warning: Possible agreement error. The noun application seems to be countable; consider using: many applications. Suggested change: "many applications" fixed Draft Text: Thus GML "gml:pos" values can be mapped directly to the YANG grouping, with the caveat that some loss of precision (in the extremes) may occur due to the YANG grouping using decimal64 values rather than doubles. Warning: Did you forget a comma after a conjunctive/linking adverb? Suggested change: "Thus," fixed Draft Text: Furthermore "gml:validTime" can either be an Instantaneous measure ("gml:TimeInstant") or a time period ("gml:TimePeriod"). Warning: Did you forget a comma after a conjunctive/linking adverb? Suggested change: "Furthermore," fixed Draft Text: As with the "kml:altitudeMode" value, the YANG grouping supports the ignore case but not the relative case. Warning: After 'the', do not use a verb. Make sure that the spelling of 'ignore' is correct. If 'ignore' is the first word in a compound adjective, use a hyphen between the two words. Note: This error message can occur if you use a verb as a noun, and the word is not a noun in standard English. I think it just wants me to hyphenate "ignore-case". "ignore" is an adjective here "case" is the noun. *shrug*, I think it reads fine the way it is for a technical document. [RW] Yes, please just ignore. The tool isn't perfect. Draft Text: Thus the YANG grouping and KML values can be directly mapped in both directions (when using a supported altitude mode) with the caveat that some loss of precision (in the extremes) may occur due to the YANG grouping using decimal64 values rather than strings. Warning: Did you forget a comma after a conjunctive/linking adverb? Suggested change: "Thus," fixed Draft Text: The allocation policy for this registry is First Come First Served, [xref] as the intent is simply to avoid duplicate values. Warning: It seems that a comma is missing. Suggested change: "Come," fixed Draft Text: All of the data nodes defined in this YANG module are writable/creatable/deletable (i.e., "config true", which is the default). Warning: Consider using all the. Suggested change: "All the" fixed Draft Text: These are the subtrees and data nodes and their sensitivity/vulnerability: None of the writable/creatable/deletable data nodes in the YANG module defined in this document are by themselves considered more sensitive or vulnerable then standard configuration. Warning: Did you mean than? Suggested change: "than" fixed Draft Text: Some of the readable data nodes in this YANG module may be considered sensitive or vulnerable in some network environments. Warning: If the text is a generality, 'of the' is not necessary. Suggested change: "Some" I think it's correct to leave this, but I could be misunderstanding the advice. [RW] I think that either is probably fine/correct. Draft Text: Below is a the YANG tree for the fictitious module that uses the geo- location grouping. Warning: Maybe you need to remove one determiner so that only a or the is left. Suggested change: "a" fixed with "the" only. Draft Text: We would also like to thank Peter Lothberg for the motivation as well as help in defining a broadly useful geographic location object, and Acee Lindem and Qin Wu for their work on a geographic location object that led to this documents creation. Warning: Possible typo: apostrophe is missing. Did you mean documents' or document's? Suggested change: "documents'" fixed. Nice tool! [RW] Thanks. It needs a bit more time and work, but the hope is that it can be an easy webtool that can be used by authors to help check the grammar/spelling in their drafts. I don't want it to be an id-nits style gatekeeper tool, because as you have seen, some of the warnings/suggestions are somewhat arbitrary. Also, the real credit goes to an open source grammar tool (dev.languagetool.org<http://dev.languagetool.org/>) that is doing all the heavy lifting. Thanks, Rob Thanks, Chris. Regards, Rob
_______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
