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

From: Michael Douglass <[email-masked]>
Date: Tue, 15 Dec 2020 12:26:15 -0500

I'm hoping to get through all your message today but I thought I'd deal 
with at least one point separately:

On 11/19/20 14:08, Barry Leiba wrote:

> This is probably a good time to pay attention to BCP 178 (RFC 6648) by
> removing the x-name construction from the ABNF (also in Section 5.1).
>
>
I agree with your point - however 6648 says

>     5.  Does not override existing specifications that legislate the use
>         of "X-" for particular application protocols (e.g., the "x-name"
>         token in [RFC5545  <https://tools.ietf.org/html/rfc5545>]); this is a matter for the designers of those
>         protocols.
So 5545 in section 3.1 defines x-name

and goes on to say:

>     Applications MUST ignore x-param and iana-param values they don't
>     recognize.
There are about 50 references to "x-".

In section 3.6 I found

> Applications MUST ignore
>     x-comp and iana-comp values they don't recognize.  Applications that
>     support importing iCalendar objects SHOULD support all of the
>     component types defined in this document, and SHOULD NOT silently
>     drop any components as that can lead to user data loss.
>
On a side note in section 3.6.6 I found this:

>           Note: Implementations should carefully consider whether they
>           accept alarm components from untrusted sources, e.g., when
>           importing calendar objects from external sources.  One
>           reasonable policy is to always ignore alarm components that the
>           calendar user has not set herself, or at least ask for
>           confirmation in such a case.
>
>
and 3.8.6.1

>   Applications MUST ignore alarms with x-name
>        and iana-token values they don't recognize.
>
3.8.8.1 and 3.8.8.2 deals with iana and x-properties

I think what I'm working round to saying is that it's probably time to 
consider a replacement for 5545. In my opinion 5545+ should state up 
front what MUST be processed and what CAN or SHOULD be ignored (though 
that may differ with the protocol used)

In the shorter term we probably at least ought to have an update which 
explicitly deprecates x- for 5545

---

From: Barry Leiba <[email-masked]>
Date: Tue, 15 Dec 2020 13:03:56 -0500

I agree that it's not up to this document to deprecate x- for
iCalendar in general, I agree that we should probably consider a
5545bis, and that we might at least consider a separate doc to
deprecate x-.  Still, I see no reason that we shouldn't *now*
eliminate "x-" from new extensions/additions that we publish.

That said, I'm not going to hold out on this point, and if the working
group thinks it's best to leave "x-" in for now, I will still move the
document forward.  But I'd like to hear *why* the working group think
that (and not just "because it's still in a lot of other places as
well").

Barry

On Tue, Dec 15, 2020 at 12:26 PM Michael Douglass
<[email-masked]> wrote:
>
> I'm hoping to get through all your message today but I thought I'd deal with at least one point separately:
>
>        token in [RFC5545]); this is a matter for the designers of those
>        protocols.
>
> I think what I'm working round to saying is that it's probably time to consider a replacement for 5545. In my opinion 5545+ should state up front what MUST be processed and what CAN or SHOULD be ignored (though that may differ with the protocol used)
>
> In the shorter term we probably at least ought to have an update which explicitly deprecates x- for 5545
>
>
>
>

---

From: Michael Douglass <[email-masked]>
Date: Tue, 15 Dec 2020 14:02:53 -0500

> eliminate "x-" from new extensions/additions that we publish.
I think it's reasonable to remove them from this spec
> that (and not just "because it's still in a lot of other places as

I think we've already moved in practice away from  x-. It's just we 
haven't explicitly said so anywhere.

Probably an agenda item for the next meeting?

---

From: Michael Douglass <[email-masked]>
Date: Tue, 15 Dec 2020 14:14:33 -0500

I should perhaps point out that many of these issues are really due to 
the practice of requiring full replacement of an entity rather than 
having an update mechanism.

SOAP for example, explicitly requires that an unrecognized (by the 
schema) element be dropped so it's a requirement to have an update 
mechanism that doesn't require a full copy of the data. The 
specification only defines what's valid.

I've been advocating for some time (while recognising the work involved) 
that we should have a data model specification that is independent of 
the representation. Other specs defining representations would then 
effectively be mappings onto the model.

OASIS - I believe - does this with their pim - a platform independent 
model. The psm is the platform specific model which defines that mapping.

They do in fact have a pim for a large subset of the calendaring model.

https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=ws-calendar#technical

---

From: Michael Douglass <[email-masked]>
Date: Tue, 15 Dec 2020 15:53:30 -0500

Just dealing with the abnf issues. Can I try out a suggestion for 
section 8.1 - The RELATED-TO abnf

On 11/19/20 14:08, Barry Leiba wrote:
> — Section 8.1 —
>
>        definition here extends the definition in Section 3.8.4.5. of
>        [RFC5545]
>
> Also here: remove the dot at the end of the section number.
>
> For the ABNF in this section I have the same comments as for the ABNF
> in Section 7.1.  In addition, this defines “relparam”, but “relparam”
> is already defined in Section 5.1.  That needs to be fixed.
>

OLD

related    = "RELATED-TO" relparam ( ":" text ) /
              (
                ";" "VALUE" "=" "UID"
                ":" uid
              )
              (
                ";" "VALUE" "=" "URI"
                ":" uri
              )
              CRLF

relparam   = *(
             ;
             ; The following are OPTIONAL,
             ; but MUST NOT occur more than once.
             ;
             (";" reltypeparam) /
             (";" gapparam) /
             ;
             ; The following is OPTIONAL,
             ; and MAY occur more than once.
             ;
             (";" other-param)
             ;
             )

NEW

related    = "RELATED-TO" relparam ( ":" text ) /
              (
                ";" "VALUE" "=" "UID"
                ":" uid
              ) /
              (
                ";" "VALUE" "=" "URI"
                ":" uri
              )
              CRLF

relparam   = ; the elements herein may appear in any order,
              ; and the order is not significant.
              [";" reltypeparam]
              [";" gapparam]
              *(";" other-param)

END

I'm not sure that this construct is still the best way to relate a VALUE 
parameter to a specific value type. It still seems to constrain the order.

Does this work better?

START

related    = "RELATED-TO" relparam ":"
              ( uid /  ; for VALUE=UID
                uri /  ; for VALUE=URI
                text ) ; for VALUE=TEXT or default
              CRLF

relparam   = ; the elements herein may appear in any order,
              ; and the order is not significant.
              [";" ("VALUE" "=" "UID" /
                    "VALUE" "=" "URI" /
                    "VALUE" "=" "TEXT")]
              [";" reltypeparam]
              [";" gapparam]
              *(";" other-param)

END

This seems to do the job - if it works I can redo the abnf for the other 
properties (and for eventpub)

---

From: Michael Douglass <[email-masked]>
Date: Thu, 31 Dec 2020 16:11:22 -0500

Thanks for your comments: I'll work through them below.

I'll submit an update in a short while.

On 11/19/20 14:08, Barry Leiba wrote:
> Hi, Michael and the working group,
>
> As we discussed the ical-relations doc during the IETF 109 session, I
> though I'd get a jump on the publication request and do an AD review
> now.  Here it is below.
>
> — Section 1 —
>
>     associated meta-data.  These relationships can take the following
>     forms
>
> Nit: That needs a colon at the end of it.
>
>        CATEGORIES are often used for this purpose but are
>        problematic for application developers.
>
> It’s a small thing, but it might be good to briefly say why it’s problematic.

Replaced with

Grouped iCalendar:  iCalendar entities can be related to each other
    as a group.  CATEGORIES are often used for this purpose but are
    problematic for application developers due to their lack of
    consistency and use as a free-form tag.

>
>     Linked:  Entities are linked to each other through typed references.
>
> Can you be a little less terse here, or explain what you mean by
> “typed references” in this context?
How about the following:

> — Section 1.1 —
>
>     and lag values and RELATED-TO is extended to allow URI values.
>
> This particular sentence would benefit from the Oxford comma after “lag values”.
Yes - done
> — Section 1.2 —
>
>     This may be, for example, to identify
>     the tasks associated with a given project without having to
>     communicate the task structure of the project, or, for example, in a
>     package delivery system all tasks associated to a specific package.
>
> Another small suggestion for rephrasing (to avoid the awkward
> repetition of “for example”):
>
> NEW
>     Two examples of how this may be used are to identify the tasks
>     associated with a given project without having to communicate the
>     task structure of the project, and to group all tasks associated to
>     a specific package in a package delivery system.
> END
Thank you - done
>   — Section 1.3 —
>
>     This more accurately
>     defines what we mean by a category.  It's not the words but the
>     meaning.
>
> Two things here:
> 1. I’m not sure what “this” refers to.  Is it “the name CONCEPT”?  Or
> is it the Simple Knowledge Organization System?
> 2. The sentence “It's not the words but the meaning,” is just sort of
> hanging around without being connected to anything.

Is this better?

The term "concept" more accurately defines what we often
mean by a category. It's not the text string that is important
but the meaning attached to it. For example, the term
"football" can mean very different sports.

> I’d really like to see this paragraph reworded.
>
>     Rather than attempt to add semantics to the current property it
>     seeems best to continue its usage as an informal tag and establish a
>     new property with more constraints.
>
> I think this would be clearer with names on the properties (and one
> "e" fewer in "seems"):
>
> NEW
>     Rather than attempt to add semantics to the existing CATEGORY
>     property, it seems best to continue its usage as an informal tag
>     and to establish a new CONCEPT property with more constraints.
> END
Done.
> — Section 1.4 —
>
>     It is often necessary to relate calendar components.
>
> To what?  To each other?  Something else?  Please say.

Does this work?

It is also often necessary to reference calendar components
in other collections. For example, a VEVENT might refer to
a VTODO from which it was derived.

> — Section 1.6 —
> Please use the new BCP 14 boilerplate and add a normative reference to RFC 8174.
Done.
> — Section 2 —
>
>     UID:  This allows for linking within a single collection and the
>        value is assumed to be another component within that collection.
>
> Why “assumed to be”?  Is this any different to “the value is the UID
> of another component within that collection”?

Changed to:

This allows for linking within a single collection and the value
MUST be another component within that collection.

> For “xpointer”, please be consistent in capitalizing the term.  I
> suggest using “XPointer” here and in Section 6, to match usage in the
> reference doc.
Done
> — Section 3 —
>
>     [RFC5988] defines two forms of relation type, registered and
>     extension.  Registered relation types are added to a registry defined
>     by [RFC5988] while extension relation types are specified as unique
>     unregistered URIs, (at least unregistered in the [RFC5988] registry).
>
> First, RFC 8288 replaced 5988 three years ago; please change to using
> that as the reference.
>
> Second, the parenthetical seems odd.  I suggest, “while extension
> relation types are specified as unique URIs that are not registered in
> the registry.”
>
> Third, I strongly suggest adding section references here.  So the
> result might be this:
>
> NEW
>     [RFC8288] defines two forms of relation type: registered and
>     extension.  Registered relation types are added to the Link
>     Relations registry as specified in Section 2.1.1 of [RFC8288].
>     Extension relation types, defined in Section 2.1.2 of [RFC8288],
>     are specified as unique URIs that are not registered in the registry.
> END
Done
> — Section 4 —
>
>     Relationship parameter type values are defined in section 3.2.15. of
>     [RFC5545].
>
> Nit: remove the dot at the end of “3.2.15” (and capitalize “Section”,
> to be consistent with how we do section references).
Done
> removing the x-name construction from the ABNF (also in Section 5.1).

I ended up reworking this section.

First I realized I didn't have to redefine the parameter anyway - just 
define new values. I broke it into 2 new sections - one defining the 
temporal values and one the rest - simply because the temporal values 
seemed to justify their own section.

>        The GAP parameter specifies the
>        lead or lag time between the predecessor and the successor.
>
> This would benefit from a forward reference: “The GAP parameter,
> defined in Section 5.2, specifies”
done
>        the description of each temporal relationship below we refer to
>        Task-A which contains and controls the relationship and Task-B the
>        target of the relationship.
>
> Nit: this needs commas: “Task-A, which contains and controls the
> relationship, and Task-B, the target”.  (There are a lot of missing
> commas throughout, and I’m not calling them all out; we’ll leave most
> to the RFC Editor.)
I'll take a read through looking for that
> — Section 5.2 —
>
>     Purpose:  To specify the length of the gap, positive or negative
>        between two temporaly related components.
>
> Another example of a missing comma that rates mention: after “negative”.
Thanks
> — Section 7.1 —
>
>        Possible category resources are the various proprietary systems,
>        for example Library of Congress, or an open source derived from
>        something like the dmoz.org data.
>
> dmoz.org died 3.5 years ago; do we really still want to use it as an example?
I have a copy of their categorizations... - but I'll cut it out of this spec
>       conceptparam = *(
> The ABNF already says that it can occur zero or more times, so the
> comment is supefluous.  I think the comments and the nested grouping
> makes this more confusing than helpful, and suggest just changing it
> to this:
>
> NEW
>       conceptparam = *(";" other-param)
> END
Agreed
>
>       SCONCEPT:http://example.com/event-types/sports
>       CONCEPT:http://example.com/event-types/arts/music
>       CONCEPT:http://example.com/task-types/delivery
>
> Is “SCONCEPT” a typo?  (And is there really value in three essentially
> indistinguishable examples?)
Yes and no - removed 2
>
> — Section 7.2 —
>
>       link            = "LINK" linkparam  /
>                         (
>                           ";" "VALUE" "=" "TEXT"
>                           ":" text
>                         )
>                         (
>                           ";" "VALUE" "=" "REFERENCE"
>                           ":" text
>                         )
>                         (
>                           ";" "VALUE" "=" "URI"
>                           ":" uri
>                         )
>                         CRLF
>
> This ABNF doesn’t make any sense.  Are you missing some alternative operators?
>
>       linkparam       = *(
>
>                       ; the following is MANDATORY
>                       ; and MAY occur more than once
>
>                       (";" relparam) /
>
>                       ; the following are MANDATORY
>                       ; but MUST NOT occur more than once
>
>                       (";" fmttypeparam) /
>                       (";" labelparam) /
>                       ; labelparam is defined in ...
>
>                       ; the following is OPTIONAL
>                       ; and MAY occur more than once
>
>                       (";" xparam)
>
>                       )
>
> And there are multiple problems here:
> - Where is labelparam defined?
> - Where is xparam defined?
> - All elements of linkparam are optional, because you use “*”, so how
> can any elements be MANDATORY?
> - The ABNF should make it clear, without comments, whether things are
> optional or mandatory and whether they can appear once or multiple
> times.
>
> Please re-work this ABNF, and get help from an ABNF expert if you need that.

OLD

link            = "LINK" linkparam  /
                   (
                     ";" "VALUE" "=" "TEXT"
                     ":" text
                   )
                   (
                     ";" "VALUE" "=" "REFERENCE"
                     ":" text
                   )
                   (
                     ";" "VALUE" "=" "URI"
                     ":" uri
                   )
                   CRLF

linkparam       = *(

                 ; the following is MANDATORY
                 ; and MAY occur more than once

                 (";" relparam) /

                 ; the following are MANDATORY
                 ; but MUST NOT occur more than once

                 (";" fmttypeparam) /
                 (";" labelparam) /
                 ; labelparam is defined in ...

                 ; the following is OPTIONAL
                 ; and MAY occur more than once

                 (";" xparam)

                 )

NEW

link           = "LINK" linkparam ":"
                    ( text /  ; for VALUE=REFERENCE
                      uri /  ; for VALUE=URI
                      text ) ; for VALUE=TEXT
                  CRLF

linkparam      = ; the elements herein may appear in any order,
                  ; and the order is not significant.

                  (";" "VALUE" "=" ("UID" /
                                    "URI" /
                                    "TEXT"))
                  1*(";" linkrelparam)
                  (";" fmttypeparam)
                  (";" labelparam)
                  *(";" other-param)

END

Also added paragraph providing reference to labelparam

>
> Also, this section is exactly one that *can* benefit from multiple
> examples, each showing a different set of value types and parameters.

Yes. Additionally, the example is incorrect. The abnf doesn't allow for 
a default value type so it should specify VALUE=URI

Also , "The Egg" is a real place. Changed to "The Venue".

>
> — Section 7.3 —
>
>     Description:  The value of this property is a text identifier that
>        allows associated components to be located or retrieved as a
>        whole.  For example all of the events in a travel itinerary would
>        have the same REFID value.
>
> This seems to be more vague than necessary.  May I suggest an update?:
>
> NEW
>     Description:  The value of this property is free-form text that
>        creates an identifier for associated components.  All components
>        that use the same REFID value are associated through that value
>        and can be located or retrieved as a group.  For example, all of
>        the events in a travel itinerary would have the same REFID value,
>        so as to be grouped together.
> END
Done.
>       refidparam      = *(
>
>                       ; the following is OPTIONAL
>                       ; and MAY occur more than once
>
>                       (";" xparam)
>
>                       )

Changed to

refidparam      = *(";" other-param)

> Same comment as with conceptparam.  And, again, where is xparam defined?
other-param (and xparam) are defined in 5545. Do we require an explicit 
reference to that spec.
> Also here: remove the dot at the end of the section number.

done. I also added the following paragraph to the description:

To preserve backwards compatibility the value type MUST be UID
when the PARENT, SIBLING or CHILD relationships are specified.

> is already defined in Section 5.1.  That needs to be fixed.

Changed the section 5.1 definition to be linkrelparam and changed its name

For related:

OLD

relparam   = *(
             ;

NEW

relparam   = ; the elements herein may appear in any order,
              ; and the order is not significant.
              [";" "VALUE" "=" ("UID" /
                                "URI" /
                                "TEXT")]
              [";" reltypeparam]
              [";" gapparam]
              *(";" other-param)

END

---

From: Barry Leiba <[email-masked]>
Date: Thu, 31 Dec 2020 16:38:05 -0500

All this looks good, and thanks.  Post the update when ready.

On your question:

> other-param (and xparam) are defined in 5545. Do we require an
> explicit reference to that spec.

I think “xparam” is not defined there (“x-param” is).  And I guess we don’t
have to be explicit, but I think it would help to have a comment that says
where it’s defined, or perhaps a general comment that some items are
defined in 5545.

Barry

On Thu, Dec 31, 2020 at 4:11 PM Michael Douglass <[email-masked]>
wrote:

> Yes. Additionally, the example is incorrect. The abnf doesn't allow for a
> default value type so it should specify VALUE=URI
>