Re: [calsify] AD review of draft-ietf-calext-ical-relations-07

[Michael Douglass <[e-mail]>]

Thanks Francesca

On 9/17/21 09:45, Francesca Palombini wrote:
> Thank you for this document. Here is my AD review.
>
> I have divided comments into "main", "minor" and "nits". I'd like to have a discussion on my main comments before requesting the IETF Last Call. On the other hand, you can address the minors and nits together with any additional Last Call comments you will get.
>
> Francesca
>
> ## main
>
> 1. -----
>
>     The LINK property SHOULD NOT be treated as just another attachment.
>
> FP: The use of SHOULD NOT instead of MUST NOT implies that there are cases where it is acceptable to treat the property as another attachment. It would be good to give implementers examples of when that's the case.
I think this should be a MUST NOT. I don't think there are any good 
reasons to use LINK as an attachment for large objects as it sidesteps 
the work on managed attachments.
>
> 2. -----
>
>     the message - for example, the agenda.  See [RFC8607]
>
> FP: I am not sure about what this reference is supposed to help the reader with. Given that this is the only downref of this document, I wonder if it is necessary to have this as normative or if it could be moved to the informational references.
This is the reference to managed attachments - I've changed the text and 
made it informative
>
> 3. -----
>
> [W3C.CR-skos-reference-20090317]
>
> FP: Would it be ok to update this to the latest version? I get a big red box telling me that this specification is outdated, but I don't know if what is used in his document changed in the newer version, hence the outdated reference.
Updated
>
> 4. -----
>
> IANA Section 11.1
>
> FP: I believe the document is missing an update to the RELATED-TO property: please add a note saying that IANA should update the RELATED-TO property in the iCalendar Properties Registry, so that the Section 9.1 of this document is added as reference (note that it should only be added, not replace RFC 5545)

Added entries for 5545 and this doc. Text now says

The following iCalendar property names have been added to the
iCalendar Properties Registry defined in Section 8.3.2 of [RFC5545]
IANA has also added a reference to this document where the properties
originally defined in [RFC5545] have been updated by this document.

and there's this entry:

| RELATED-TO | Current | [RFC5545], Section 9 |

Does this work?

>
> ## minor
>
> 5. -----
>
>     The ATTACH property is being extended to handle server-side
>
> FP: The ATTACH property is mentioned here for the first time. Please add a reference to where it's defined.
Done
>
> 6. -----
>
>     the RELTYPE parameter redefined in Section 3.2.15 of [RFC5545]:
>
> FP: why "redefined" and not "defined"?
Typo - fixed
>
> 7. -----
>
>     Conformance:  This property MAY be specified in any iCalendar
>        component.
>
> FP: for section 8.2 and 9.1, the Conformance above does not specify if the property may be specified more than once. I would think it is allowed for both LINK and RELATED-TO, but it would be better if it was clearly defined.

Replaced conformance text in LINK and REFID to be the same as for 
CONCEPT - that is:


This property can be specified zero or more times in any
iCalendar component.

>
> 8. -----
>
>     RELTYPE=FINISHTOFINISH:  Task-B cannot finish before Task-A is
>        finished, that is the end of Task-A defines the end of Task-B.
>
> FP: I don't think the example just after, mentioning that all tasks must be done at the same time, showcases the definition quoted above: from the definition above, if A is meat and B is potatoes, the only thing we can derive from the above relationship is that meat must be cooked before potatoes are cooked. I don't see anything excluding that A can finish before B is started.

I had to read the definitions again. How about:

=== OLD

Task-B cannot finish before Task-A is
    finished, that is the end of Task-A defines the end of Task-B.
    For example, we start the potatoes, then the meat then the peas
    but they should all be cooked at the same time.
=== NEW
Task-B can only be completed after Task-A is
    finished.  The related tasks may run in parallel before
    completion.

    For example, if the goal is to prepare a meal, we start the
    potatoes, then the meat then the peas but they should all be
    cooked at the same time.
=== END

I also changed the diagram to show the tasks may be of different lengths

>
> ## nits
>
> 9. -----
>
>     The link property can link components in different collections or
>
> FP: I believe the first "link" should be "LINK".
>
I've submitted version 8