Just posted this new version. > On Apr 13, 2021, at 6:08 AM, Rob Wilton (rwilton) <[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]> >> 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
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
