Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tom Petch all comments #144

Open
8 of 9 tasks
sergiobelotti opened this issue Jul 3, 2023 · 1 comment
Open
8 of 9 tasks

Tom Petch all comments #144

sergiobelotti opened this issue Jul 3, 2023 · 1 comment
Assignees
Labels
draft text update Requires an update of the I-D text IETF-120

Comments

@sergiobelotti
Copy link
Collaborator

sergiobelotti commented Jul 3, 2023

Please see the email with Tom comments
https://mailarchive.ietf.org/arch/msg/ccamp/RIo9U_DQpo4CFFwgOyhN4YV1a88/

Prompted by the YANG doctor review I had another look at this and have some additional comments

References - lots needed in the YANG; import MUST have them and the data should in many cases. The Informational References in the I-D look good but need adding to the YANG.

s.1.3 layer 0 prefix is incorrect - the module has it right.

Fixed

Description I would like more especially where it is '..'

Look too at the prefix of the referenced modules - tet, nw ne - good choices; optical-imp-topo is too long IMHO; IANA has a list of modules and prefix most of which look good. You might want to share some initial letters with other CCAMP modules (WDM?).

  • To be discussed - Agreed . YANG and TEXT in the draft
  • Agreed to use "oit" as prefix.

There are some quirky spellings but I leave those a last call review

Figure 1 title would be better as RFC YYYY ....
with a note to the RFC Editor

  • To be fixed- TEXT in the draft

There are a number of TBA TBD yet to be fleshed out

  • To be fixed TEXT in the draft

Compared to earlier revisions, many of the 'augment' 'when' have been amended to relative form but not all; I am sure you know the impact of absolute form but is this intended?

In the descriptions I think '1' should be 'one'

           leaf nominal-power-spectral-density{
               type decimal64 {
                   fraction-digits 16;
             units W/Hz ;
             description
                 Typical value : 3.9 E-14, resolution 0.1nW/MHz"; That typical value like any value will look ugly with 16 fraction digits - I assume that this does not use floating point.

This is a common issue with uint64 and decimal64. The bandwidth-ieee-float32 defined in RFC8294 would be even more ugly.

Security Considerations is out of date; current one drags in HTTPS, TLS, RESTCON.

Tom Petch

@italobusi italobusi added draft text update Requires an update of the I-D text YANG model update labels Jul 3, 2023
italobusi added a commit that referenced this issue Jul 3, 2023
Co-authored-by: sergio belotti <sergio.belotti@nokia.com>
@sergiobelotti
Copy link
Collaborator Author

sergiobelotti commented Sep 12, 2023

Look too at the prefix of the referenced modules - tet, nw ne - good choices; optical-imp-topo is too long IMHO; IANA has a list of modules and prefix most of which look good. You might want to share some initial letters with other CCAMP modules (WDM?).

To be discussed

Call on September 12:
one proposal could be "oit".

@sergiobelotti sergiobelotti added IETF-118 update for IETF-118 and removed IETF-117 labels Oct 2, 2023
dieterbeller added a commit that referenced this issue Oct 20, 2023
This I-D update addesses:

- #144 : some comments resolved
- #153 : new DGE sub-section added
- #154 : latest YANG code and tree view incorporated

+ pure editorial updates/corrections

Co-Authored-By: Esther Le Rouzic <EstherLerouzic@users.noreply.github.com>
Co-Authored-By: sergio belotti <sergio.belotti@nokia.com>
Co-Authored-By: italobusi <12461741+italobusi@users.noreply.github.com>
Co-Authored-By: Gabriele Galimberti <ggalimbe56@gmail.com>
italobusi added a commit that referenced this issue Oct 23, 2023
Fixed relative paths in grouping media-channel-groups: see comment from Tom Petch in #144

Added muxponder constraints: fix #120

Added DGE attributes to amplifier element: see #153 (comment)

Changed the type of delta-power

Aligned with ietf-ccamp-wg/ietf-ccamp-layer0-types-ext-RFC9093-bis#78

---------

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>
dieterbeller added a commit that referenced this issue Oct 23, 2023
* I-D text update resolving #150 and #151

This I-D update resolves:

- #150
- #151

+ pure editorial updates/corrections

Co-Authored-By: Esther Le Rouzic <EstherLerouzic@users.noreply.github.com>
Co-Authored-By: Gabriele Galimberti <ggalimbe56@gmail.com>
Co-Authored-By: sergio belotti <sergio.belotti@nokia.com>
Co-Authored-By: italobusi <12461741+italobusi@users.noreply.github.com>

* I-D text update addressing  #144, #153, #154

This I-D update addesses:

- #144 : some comments resolved
- #153 : new DGE sub-section added
- #154 : latest YANG code and tree view incorporated

+ pure editorial updates/corrections

Co-Authored-By: Esther Le Rouzic <EstherLerouzic@users.noreply.github.com>
Co-Authored-By: sergio belotti <sergio.belotti@nokia.com>
Co-Authored-By: italobusi <12461741+italobusi@users.noreply.github.com>
Co-Authored-By: Gabriele Galimberti <ggalimbe56@gmail.com>

* I-D text update resolving #157

This I-D update resolves:

- #157 : Examples of how vendors can define their operational modes.

+ pure editorial updates/corrections

Co-Authored-By: Esther Le Rouzic <EstherLerouzic@users.noreply.github.com>
Co-Authored-By: sergio belotti <sergio.belotti@nokia.com>
Co-Authored-By: italobusi <12461741+italobusi@users.noreply.github.com>
Co-Authored-By: Gabriele Galimberti <ggalimbe56@gmail.com>

---------

Co-authored-by: Esther Le Rouzic <EstherLerouzic@users.noreply.github.com>
Co-authored-by: Gabriele Galimberti <ggalimbe56@gmail.com>
Co-authored-by: sergio belotti <sergio.belotti@nokia.com>
Co-authored-by: italobusi <12461741+italobusi@users.noreply.github.com>
@italobusi italobusi removed YANG model update IETF-118 update for IETF-118 labels Dec 1, 2023
@sergiobelotti sergiobelotti added the IETF-119 Updates for IETF 119 label Feb 13, 2024
@sergiobelotti sergiobelotti added IETF-120 and removed IETF-119 Updates for IETF 119 labels Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft text update Requires an update of the I-D text IETF-120
Projects
None yet
Development

No branches or pull requests

3 participants