Hi Per, Thanks for the review. Please see inline. Cheers, Med (as author) > -----Message d'origine----- > De : Per Andersson via Datatracker <noreply@xxxxxxxx> > Envoyé : vendredi 27 juin 2025 22:33 > À : ops-dir@xxxxxxxx > Cc : draft-ietf-netmod-schedule-yang.all@xxxxxxxx; last- > call@xxxxxxxx; netmod@xxxxxxxx > Objet : draft-ietf-netmod-schedule-yang-07 ietf last call Opsdir > review > > > Document: draft-ietf-netmod-schedule-yang > Title: A Common YANG Data Model for Scheduling > Reviewer: Per Andersson > Review result: Has Nits > > Reviewer: Per Andersson > Review Result: Has Nits > > I have reviewed this document as part of the Operational > directorate's ongoing effort to review all IETF documents being > processed by the IESG. > These comments were written with the intent of improving the > operational aspects of the IETF drafts. Comments that are not > addressed in last call may be included in AD reviews during the IESG > review. Document editors and WG chairs should treat these comments > just like any other last call comments. > > > Summary: > > The document defines common types and groupings for scheduling > purposes. > A YANG module is defined which includes a set of recurrence related > groupings with basic to advanced levels of representation. Groupings > for validating schedules and reporting schedule status are also > defined. > > I find the document easy to read and to understand, with a good > presentation of the proposed solution. > > > Major issues: > > There are no major issues. > > > Minor issues: > > * Section 3.3.6 > > For readers unaware of the yang:timeticks type definition a > reference > to RFC 6991 for the period-timeticks using the yang:timeticks type > could be good. > [Med] Sure. Fixed. > > * Section 3.3.8 > > Why is the iCalendar BYWEEKNO, BYMONTH, and WKST defined as > "byyearweek", "byyearmonth", and "workweek-start" respectively? > Suggest to follow the naming convention from RFC 5545, e.g. > "byweekno", "bymonth", and "week-start". > [Med] The 5545 labels for these ones do not adequately reflect their actual definition and preferred to use more meaningful ones inspired by the definition. For example, we went for " workweek-start" for WKST rather than "week-start" because 5545 says: "The WKST rule part specifies the day on which the workweek starts". Idem for the other two. > > * Section 3.3.9 > > Why have a leaf with current local time instead of the pattern > that > was used before with yang:date-and-time leafs and a > timezone-identifier if the times are in local time? This incurs > different calculations to be made to infer the time offset from > UTC > for the schedule-status groupings and the other groupings. In one > the > timezone is supplied and can be feed to a time generating function > to > calculate the localtime, in the other the current UTC timestamp is > diffed from the supplied localtime to calculate what timezone (or > offset from UTC) is used. > > Suggest to use the pattern with timezone-identifier even for the > schedule-status groupings. > [Med] Please refer to https://github.com/netmod-wg/schedule-yang/pull/26 for the rationale for that structure (basically, follow RFC3231). > > * Section 6 > > RFC 6991 is listed as a normative reference. However the numerical > values in the must statement in period-start are magical constants > in > the YANG module without any further explanation. Suggest to add a > reference and elaborate what the leaf values represent, instead of > just stating "start/stop time". > > [Med] Not sure what the description would say here more than this indicates when a period starts ;-) If you have a particular change in mind, please share it. > Nits: > > The following commands yield a diff > > $ pyang -f yang --keep-comments --yang-line-length=69 \ > ietf-schedule@xxxxxxxxxxxxxxx > new.yang $ diff ietf- > schedule@xxxxxxxxxxxxxxx new.yang [Med] Fixed. Thanks. > > 11d10 > < > 283a283 > > > 439,440c439 > < error-message > < "Frequency must be provided."; > --- > > error-message "Frequency must be provided."; > 594,611c593,609 > < must > < "(not(derived-from(../../frequency," > < +"'schedule:secondly')) or (current() < 100)) and " > < +"(not(derived-from(../../frequency," > < +"'schedule:minutely')) or (current() < 6000)) and " > < +"(not(derived-from(../../frequency,'schedule:hourly'))" > < +" or (current() < 360000)) and " > < +"(not(derived-from(../../frequency,'schedule:daily'))" > < +" or (current() < 8640000)) and " > < +"(not(derived-from(../../frequency,'schedule:weekly'))" > < +" or (current() < 60480000)) and " > < +"(not(derived-from(../../frequency," > < +"'schedule:monthly')) or (current() < 267840000)) and " > < +"(not(derived-from(../../frequency,'schedule:yearly'))" > < +" or (current() < 3162240000))" { > < error-message > < "The period-start must not exceed the frequency > < interval."; > --- > > must "(not(derived-from(../../frequency," > > + "'schedule:secondly')) or (current() < 100)) and " > > + "(not(derived-from(../../frequency," > > + "'schedule:minutely')) or (current() < 6000)) and " > > + "(not(derived- > from(../../frequency,'schedule:hourly'))" > > + " or (current() < 360000)) and " > > + "(not(derived- > from(../../frequency,'schedule:daily'))" > > + " or (current() < 8640000)) and " > > + "(not(derived- > from(../../frequency,'schedule:weekly'))" > > + " or (current() < 60480000)) and " > > + "(not(derived-from(../../frequency," > > + "'schedule:monthly')) or (current() < 267840000)) and > " > > + "(not(derived- > from(../../frequency,'schedule:yearly'))" > > + " or (current() < 3162240000))" { > > error-message > > "The period-start must not exceed the frequency > > interval."; > 674,675c672,673 > < + "(derived-from(../../frequency, 'schedule:yearly') " > < + " and not(../../byyearweek))"; > --- > > + "(derived-from(../../frequency, 'schedule:yearly') " > > + " and not(../../byyearweek))"; > > > Spelling: > > iclander -> iCalender > occurences -> occurrences > avaliable -> available > paramter -> parameter > prioritised -> prioritized > specifc -> specific > maxinum -> maximum > difficient -> difficult (?) > feaure -> feature > YANGDOCTORS -> YANG Doctors [Med] Fixed all. Thanks. > > > Thanks for your contribution! > > > -- > Per > ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
