Re: [CCAMP] Should the GitHub repo mentioned in the TNBI draft appendix moved under CCAMP WG's GitHub (was RE: [RTG-DIR] Rtgdir early review of draft-ietf-ccamp-transport-nbi-app-statement-12)

[Dieter Beller <[e-mail]>]

Hi Italo, all,

> Instead of moving the personal GitHub under CCAMP WG’s GitHub, it 
> might be *better to fork from the personal GitHub to the CCAMP WG’s 
> GitHub*. In this way, the owner of the personal GitHub can freely 
> continue working on his tool and the CCAMP WG can, later, decide 
> whether or not to synch-up with these changes.

I concur. Here is my opinion without having checked whether there are 
some IETF recommendations describing what to do in such a case:

Forking from a personal GitHub repo is probably the the better choice. 
Important is IMHO that the full repo history is available on GitHub 
including the one before WG adoption,
which usually is the point in time when the repo should be put onto the 
WG's GitHub.


Thanks,
Dieter

On 30.09.2021 11:33, Italo Busi wrote:
>
> Hi CCAMP WG,
>
> We are addressing Dhruv’s comments on the TNBI draft but we think this 
> comment requires further discussion from the WG:
>
> - Should the GitHub repo mentioned in the appendix moved under CCAMP 
> WG's GitHub? That would be giving some control over the longtime 
> validity of these URLs. Should they be added as references in the 
> document as well.
>
> The comment looks reasonable to us
>
> Instead of moving the personal GitHub under CCAMP WG’s GitHub, it 
> might be better to fork from the personal GitHub to the CCAMP WG’s 
> GitHub. In this way, the owner of the personal GitHub can freely 
> continue working on his tool and the CCAMP WG can, later, decide 
> whether or not to synch-up with these changes.
>
> What do you think?
>
> Thanks, Italo (on behalf of co-authors)
>
> *From:* Daniel King [mailto:[e-mail]] *On Behalf Of 
> *[e-mail]
> *Sent:* giovedì 13 maggio 2021 14:16
> *To:* 'Dhruv Dhody' <[e-mail]>; 'Dhruv Dhody' 
> <[e-mail]>
> *Cc:* [e-mail]; 'CCAMP' <[e-mail]>; 
> [e-mail]
> *Subject:* RE: [RTG-DIR] Rtgdir early review of 
> draft-ietf-ccamp-transport-nbi-app-statement-12
>
> Hi Dhruv,
>
> Thanks very much for the detailed review! The comments and suggestions 
> are most welcome.
>
> Re: Draft Naming
>
> Let me chat with the co-authors and get back to you and the WG with a 
> response.
>
> For the other items, we will address in the next version.
>
> BR, Dan.
>
> *From:* Dhruv Dhody <[e-mail] <mailto:[e-mail]>>
> *Sent:* 11 May 2021 12:38
> *To:* Dhruv Dhody <[e-mail] <mailto:[e-mail]>>
> *Cc:* [e-mail] <mailto:[e-mail]>; CCAMP 
> ([e-mail] <mailto:[e-mail]>) <[e-mail] 
> <mailto:[e-mail]>>; 
> [e-mail] 
> <mailto:[e-mail]>
> *Subject:* Re: [RTG-DIR] Rtgdir early review of 
> draft-ietf-ccamp-transport-nbi-app-statement-12
>
> Hi,
>
> The formatting seems to be off when I uploaded the text file in the 
> review tool. It looks okay in the datatracker : 
> https://datatracker.ietf.org/doc/review-ietf-ccamp-transport-nbi-app-statement-12-rtgdir-early-dhody-2021-05-11/ 
> <https://datatracker.ietf.org/doc/review-ietf-ccamp-transport-nbi-app-statement-12-rtgdir-early-dhody-2021-05-11/> 
> ; seems to be an issue with the email then!
>
> I am pasting the review again, hoping this works!
>
> =====
>
> Subject:RtgDir Early review: draft-ietf-ccamp-transport-nbi-app-statement
>
> Hello
>
> I have been selected to do a routing directorate “early” review of 
> this draft.
> https://datatracker.ietf.org/doc/draft-ietf-ccamp-transport-nbi-app-statement/ 
> <https://datatracker.ietf.org/doc/draft-ietf-ccamp-transport-nbi-app-statement/>
>
> The routing directorate will, on request from the working group chair, 
> perform an “early” review of a draft before it is submitted for 
> publication to the IESG. The early review can be performed at any time 
> during the draft’s lifetime as a working group document. The purpose 
> of the early review depends on the stage that the document has reached.
>
> This review request might have been marked as an Early review by 
> mistake. The review stands nevertheless.
>
> For more information about the Routing Directorate, please see 
> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir 
> <http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir>
>
> Document: draft-ietf-ccamp-transport-nbi-app-statement
> Reviewer: Dhruv Dhody
> Review Date: 2021-05-10
> Intended Status: Informational
>
> Summary:
> I have some minor concerns about this document that I think should be 
> resolved before it is submitted to the IESG.
>
> Overview:
> I have done a review of this I-D based on my understanding of ACTN and 
> the related YANG models. Overall, I found the document difficult to 
> review. I was scrolling up and down the document and finally had to 
> keep the document open on multiple screens to cross-reference various 
> figures to understand the text. I wonder if there is a way to improve 
> readability; can't think of anything obvious...
>
> Given the complexity of the subject matter, I do believe that the 
> authors have done a good job. I hope the document would be useful for 
> the implementors.
>
> I have some concerns about the JSON Code which I have listed first, 
> followed by other minor comments and nits!
>
> JSON Code Issues:
> - This document provides a 'non-standard' JSON Code in the appendix 
> and provides personal GitHub links on how to validate that JSON code. 
> While the JSON code is common in the YANG documents, this document 
> introduces what looks like a new way to add comments. Question to John 
> (as responsible AD): Does this approach has any IETF process issue? 
> The authors have done a decent job in explaining the need for it and 
> how to handle it.
> - I tried to set up the docker validate tool as per 
> https://github.com/GianmarcoBruno/json-yang 
> <https://github.com/GianmarcoBruno/json-yang> ; I was not able to make 
> it run and gave up after a while. :( The instruction on GitHub 
> README.md does not match with what's in the docker image include the 
> options.
> - JSON code is based on an older version of YANG models. It would be a 
> good idea to update it to the latest. Related question: YANG models 
> are normative references (which seems the right thing to do), so this 
> document should be published together or after those YANG models are 
> ready and thus some coordination across WG is required. Again 
> something for John to consider.
> - JSON code should start with <CODE BEGINS>. Example - <CODE BEGINS> 
> file "mpi1-otn-topology.json", so that the json code can be stripped 
> from the document.
> - Should the GitHub repo mentioned in the appendix moved under CCAMP 
> WG's GitHub? That would be giving some control over the longtime 
> validity of these URLs. Should they be added as references in the 
> document as well.
> - Update this -
>   ========== NOTE: '\\' line wrapping per BCP XXX (RFC XXXX) ==========
>   It was published as an informational RFC 8792, so remove the BCP and 
> update the RFC number!
> - On first reading I did not understand the TTP ID naming convention 
> used as per
>   "// __COMMENT__ tunnel-tp-id": "AN1-1 TTP-ID (1 ->\
>    \ 0x01 -> 'AQ==')"
>   Perhaps it could be mentioned somewhere that AQ== is the base64 
> representation of 0x01.
> - Appendix B.1.1, Figure 3 - "OTN Abstract Topology exposed at MPI1 
> (MPI1 OTN Topology)" does not have AN1-8 but the JSON does. I think 
> that might be a mistake?
>
>
> Others:
> - Title, "Transport Northbound Interface Applicability Statement" - 
> not sure about the title, it does not match with the content of the I-D.
> - Section 2, the definition of connection is not clear. The use of 
> connection/LSP multiple times is also distracting. Similarly "Link: A 
> link, or a physical link, ..." reads weird.  The note in section 2 
> needs to be handled at this stage. Note that [TE-TUTORIAL] is already 
> marked informative in the -12 version!
> - Section 3.1, the notations need to explain what {} means.
> - Section 3.2 is better suited for the appendix as the JSON code 
> exists only in the appendix.
> - Section 4.6 is not clear to me. Should one refer to RFC 8640, 8641, 
> and 8650 as far as the datastore update is concerned? Clarify the term 
> 'alarm', do you mean YANG notification or RFC 8623? This section needs 
> some work.
> - Section 5.1.1, the text at the very end of this section about JSON 
> code is much useful closer to the code in the appendix.
> - Section 5.1.4, we need to all add some text to describe for merging 
> of multiple instances of TE topology from the same PNC. For example, 
> the merging of OTN (Figure 3) and ETH (Figure 4) should lead to 
> Network domain 1 topology in Figure 6.
> - Section 5.2.1, is there any reference that can be provided about the 
> handling of TTP and route-object-include-exclude? Or is this document 
> specifying how they should be handled? This is applicable in other 
> instances as well.
> - Section 5.2.2, "...abstracting S3-1 and S18-3 TTPs", please show 
> S18-3 in the figure, otherwise, it is not clear which TTP it is!
>
> Nits:
> - Authors address at the end of the document do not match the front
> - Please make abstract as the first section as per the style guide 
> https://www.rfc-editor.org/rfc/rfc7322.html#section-4 
> <https://www.rfc-editor.org/rfc/rfc7322.html#section-4>
> - It is expected to use https in the status of the memo
> - Expand on first use: ODU, EPL, EVPL, OTN, LSP, FC, STM-n, L1CSM, 
> L2SM, VN,
> - Section 2, s/swith/switch/ ; s/failurs/failures/
> - Section 2, adding a reference to documents where these terms come 
> from would help. It's already done for some terms!
> - Section 4.2, s/[RFC8453] Provides/[RFC8453] provides/ ; s/PNcs/PNCs/
> - Section 4.3, Ri (PKT -> foo) and Rj (foo -> PKT) does not align to 
> the format in Section 3.1. Should this be Ri [(PKT) -> foo] and Rj 
> [foo -> (PKT)] instead?
> - Section 4.7, order of closing brackets is incorrect
> OLD
>       R1 [(PKT) -> ODU2], S3[(ODU2]), S1 [(ODU2]), S2[(ODU2]),
>       S8 [(ODU2]), S12[(ODU2]), S15 [(ODU2]), S18[(ODU2]), R8 [ODU2 ->
>       (PKT)]
> NEW
>       R1 [(PKT) -> ODU2], S3[(ODU2)], S1 [(ODU2)], S2 [(ODU2)],
>       S8 [(ODU2)], S12[(ODU2)], S15 [(ODU2)], S18[(ODU2)], R8 [ODU2 ->
>       (PKT)]
> - Section 5.2, s/models is (e.g., L1CSM, L2SM, VN) is outside/models 
> (e.g., L1CSM, L2SM, VN) is outside/
> - Section 5.2, The text says ".. (steps 2.1, 2.2 and 2.3 in Figure 
> 7)". There are no steps 2.1, 2.2, and 2.3 in the figure.
> - Section 5.2.1, s/OUD2/ODU2/
> - Section 5.2.1.1, s/terminating on AN-1 and AN1-2 LTPs/terminating on 
> AN1-1 and AN1-2 LTPs/
> - Section 5.2.2, s/Appendix Error! Reference source not 
> found./Appendix B.2.3/; s/OUD2/ODU2/; s/[ETH -> (ODU)]/[ETH -> (ODU2)]/g;
> - Section 5.2.2.1, s/Tunnel between the AN1-1 and AN1-2/Tunnel between 
> the AN1-1 and AN1-8/
> - Section 5.2.3, s/[STM-64 -> (ODU)]/[STM-64 -> (ODU2)]/
> - Section 5.2.4, "..physical nodes S3, S1, S2, S31, S33, S15 and 
> S18.." -> S34 is missing!!!
> - Section 5.3.2, s/S31 and D34/S31 and S34/ ; s/lin/link/ ;
> - Appendix A, s/an Internet-Draft/this document/
>
>
> Thanks!
> Dhruv
>
>
> _______________________________________________
> CCAMP mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ccamp