Thanks for the review Peter, and for the updates, Vinay! I do have a remaining question about the copyright. Do we have an answer? Jari On 21 Apr 2016, at 04:44, vinays <vinays@xxxxxxxxx> wrote: > > > Sent too soon. Please see inline at VC2. > > Thanks, > -Vinay. > > > On 4/20/16 10:41 PM, vinays wrote: >> >> >> Have addressed the MIB related comments below. The remaining will be addressed >> by Tim in the latest revision to be submitted tomorrow morning UK time. >> >> Please see inline at VC. >> >> >> On 4/17/16 4:18 PM, Peter Yee wrote: >>> I am the assigned Gen-ART reviewer for this draft. The General Area Review >>> Team (Gen-ART) reviews all IETF documents being processed by the IESG for >>> the IETF Chair. Please wait for direction from your document shepherd or >>> AD before posting a new version of the draft. >>> >>> For more information, please see the FAQ at >>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. >>> >>> >>> Document: draft-ietf-tictoc-ptp-mib-08 >>> Reviewer: Peter Yee >>> Review Date: March 8, 2016 >>> IETF LC End Date: March 8, 2016 >>> IESG Telechat date: April 21, 2016 >>> >>> Summary: This draft is basically ready for publication as a Standards >>> Track RFC, but has nits and a couple of minor issues that should be >>> fixed/considered before publication. [Ready with issues] >>> >>> This draft defines an SMIv2 MIB for use with the Precision Time Protocol >>> v2 (IEEE Std. 1588-2008). I do not have access to this standard, so my >>> review primarily addresses nits within the module and not whether the >>> module itself is a fair representation of a MIB suitable for use with PTP >>> v2. >>> >>> The following review was originally submitted for the LC (see >>> http://www.ietf.org/mail-archive/web/gen-art/current/msg13014.html). As >>> no update has been issued for the draft, the content of that review is >>> copied below. >>> >>> Major issues: None >>> >>> Minor issues: >>> >>> Given that the draft seems to quote a fair amount of text (at least 5 >>> pages) from IEEE Std. 1588-2008 in the definitions section if not >>> elsewhere, it might be good to make sure that there are no copyright >>> issues. >>> >>> Many of the object descriptions use identical text. While that's helpful >>> from the direct lookup point of view, it doesn't help to distinguish these >>> items from others of their ilk. I'm not sure if this matters, but perhaps >>> enhancing the descriptions in their contexts might make them more useful. >>> >>> Nits: >>> >>> General: >>> >>> Replace all occurrences of "create logical group" with "create a logical >>> group". >> >> VC: done. >> >>> >>> Watch the case and spacing of object names in their descriptions. >>> Sometime there's a space between two words, sometimes not. Some words are >>> capitalized in one place and not another. Use of camel case conventions >>> comes and goes. It's probably best to treat the object names as English >>> words in the descriptions, separating words with spaces and generally >>> writing them in lower case. Sure, the inconsistency doesn't hurt a lot, >>> but it ends up looking a wee bit sloppy. The RFC Editor may not be sure >>> how to handle the inconsistency as some word combinations might appear to >>> be technical MIB terms that should be left as is. >>> >>> Drop the draft name from the center, top header. Generally a shortened >>> form of the draft title is placed here. Perhaps you could use "PTPv2 MIB"? >>> >>> I'll leave the serial comma be. It appears in some places and not others. >>> I like them, but there are enough missing ones that I'm just going to let >>> it slide. ;-) >>> >>> When citing the official name of IEEE 1588-2008, use a lower case "a” >>> before "Precision". That aligns with IEEE's title. >>> >>> The authors' addresses in Section 9 have way too many punctuation marks in >>> them. A few less commas would go a long way. Maybe change some into >>> serial commas for sprinkling elsewhere in the document? ;-) >>> >>> Specific: >>> >>> Page 2, Section 1, 1st paragraph, 2nd sentence: delete "the” before >>> "ordinary". Make "ordinary clock" and "transparent clock" plural. Insert >>> a comma and "and" after "transparent clocks". >>> >>> Page 3, Section 1.1, 1st paragraph, 2nd sentence: append a comma after >>> "e.g.". >>> >>> Page 5, 1st bullet item: delete the comma after "STD 62". >>> >>> Page 5, 2nd bullet item, 2nd sentence: insert "is" before "described". >>> >>> Page 5, 3rd bullet item, 2nd sentence: insert "is" before "described". >>> >>> Page 5, 3rd bullet item, last sentence: insert "is" before "described". >>> >>> Page 7, MIB Description, 1st paragraph, 1st sentence: change "MIB is to >>> support" to "MIB supports". >> VC: done. >> >>> Page 8, Acronyms, EUI: delete the period after the acronym expansion. >> >> VC: done. >> >> >>> >>> Page 12, PTP node definition: insert "A" before the sentence. >> >> VC: done. >> >>> >>> Page 16, ClockIdentity description: the term "MAC-48" has not been defined >>> or previously referenced in the draft. It's also not quite an exact >>> synonym for EUI-48, so perhaps it should be defined. >> >> VC: done. Have added MAC-48 in the acronym section. >> >>> >>> Page 17, ClockMechanismType description: replace "End to End” with >>> "end-to-end". Replace "peer to peer" with "peer-to-peer". >> >> VC: done. >> >> >>> >>> Page 19, ClockPortTransportTypeAddress description, 2nd sentence: change >>> "Transport" to "transport". >> >> VC: done. >> >> >>> >>> Page 19, ClockPortTransportTypeAddress description, 4th sentence: insert >>> "an" before "address". Change "and" to "or". >> >> VC: done. >> >> >>> >>> Page 20, ClockQualityAccuracyType description: change "section” to >>> "sections". Insert "of" before "RFC 2578", which should be placed in >>> square brackets. Change "to start with" to "starting at". >> >> VC: done. >> >>> >>> Page 23, ClockStateType description: insert "a" before "PTP". >> >> VC: done. >>> >>> Page 24, Holdover state text: change "ptp" to "PTP. Change "FREERUN” to >>> "Freerun". >>> > > VC2: done. > > >>> Page 25, ClockTimeSourceType description: change "section" to "sections”. >>> Insert "of" before "RFC 2578", which should be placed in square brackets. >>> Change "to start with" to "starting at". > > VC2: done. > >>> >>> Page 26, ClockTimeSourceType reference: change "Section" to "Sections". > > VC2: done. > > >>> >>> Page 32, ptpbaseClockCurrentDSOffsetFromMaster and >>> ptpbaseClockCurrentDSMeanPathDelay references: the indentation looks a bit >>> off here. > > VC2: done. > > >>> >>> Page 32, ptpbaseClockCurrentDSMeanPathDelay description: change "measure” >>> to "measured". > > VC2: done. > > >>> >>> Page 35, ptpbaseClockParentDSOffset description (near top of page): change >>> "clocks" to "clock's". > VC2: done. > >>> >>> Page 35, ptpbaseClockParentDSClockPhChRate description: change "clocks” to >>> "clock's". > > VC2: done. > > >>> >>> Page 41, ptpbaseClockRunningState description: insert "a" before "PTP”. >>> Append a comma after "engine". > > VC2: done. > > >>> >>> Page 42, top: Rather than duplicate the state descriptions information >>> here, why not merely point back to ClockStateType? Btw, PTP engine hasn't >>> been defined. > > VC2: done. > >>> >>> Page 42, Holdover state, 1st sentence: if you do not remove this paragraph >>> as suggested in the previous comment, change "ptp" to "PTP". >>> >>> Page 42, ptpbaseClockRunningPacketsSent description: replace "packet >>> Unicast and multicast" with "unicast and multicast packets". >>> >>> Page 42, ptpbaseClockRunningPacketsReceived description: replace “packet >>> Unicast and multicast" with "unicast and multicast packets". >>> >>> Page 44, ptpbaseClockTimePropertiesDSCurrentUTCOffsetValid description: >>> insert "the" before "UTC". >>> >>> Page 45, ptpbaseClockTimePropertiesDSCurrentUTCOffset description (at top >>> of page): insert "the" before "UTC". >>> >>> Page 49, ptpbaseClockTransDefaultDSDelay description (at top of page): >>> change "shall be" to "of". Change "If" to "if". >>> >>> Page 56, ptpbaseClockPortRunningTable description: should that be “a >>> dataset" or "datasets". Plain "dataset" doesn't work. >>> >>> Page 57, ptpbaseClockPortRunningEntry description: change "runing” to >>> "running" unless Viking writing is involved. >>> >>> Page 58, ptpbaseClockPortRunningState description: you probably don't need >>> to include the states here since they are already found in the definition >>> of ClockPortState. >>> >>> Page 60, ptpbaseClockPortRunningEncapsulationType description: change >>> "eg.” to "e.g.,". >>> >>> Page 60, ptpbaseClockPortRunningRxMode description: change "specifie” to >>> "specifies". >>> >>> Page 63, ptpbaseClockPortTransDSPeerMeanPathDelay description: delete the >>> parentheses and append a comma after PTP. On the following page, change >>> "value is" to "value of". >>> >>> Page 64, ptpbaseClockPortAssociateTable description: change "which” to >>> "that". >>> >>> Page 75, ptpbaseMIBClockPortTransDSGroup description: does “ TransparentDS >>> Dataset" contain a redundancy? > > VC2: All the above are done as well. Thanks, > -Vinay. > > > >>> >>> Page 76, 4th paragraph, 1st sentence: delete "the" before "implementers”. >>> Delete "as". >>> >>> Page 76, 5th paragraph, 1st sentence: change "Further" to "Furthermore". >>> >>> >> > > _______________________________________________ > Gen-art mailing list > Gen-art@xxxxxxxx > https://www.ietf.org/mailman/listinfo/gen-art
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail