Skip to content

Conversation

sideshowbarker
Copy link
Member

There are several instances in the spec of this pattern:

The following fields MUST be included in a FOO for it to be valid

One problem with those as normative statements is that the spec doesn’t explicitly define what ”to be valid” means or what must happen if an instance of something is found to not be “valid”.

But it doesn’t really matter because those statements are anyway just redundant with the requirements that are already normatively specified in the associated WebIDL definitions.

For example, the WebIDL for PaymentCurrencyAmount already normatively specifies the currency & value fields as required, so the normative statement after it that “The following fields must be supplied for a PaymentCurrencyAmount to be valid“ is redundant with what’s specified in the WebIDL.

So this PR replaces those redundant normative “The following fields MUST be included in a FOO for it to be valid:” instances with just the non-normative “The following fields are required:”

@adrianhopebailie
Copy link
Collaborator

LGTM

@halindrome
Copy link
Contributor

I don't have a problem with this specific change. Two comments:

First, we should carefully identify requirements on the user vs. requirements on the implementor. When writing tests, we want to focus on the requirements on the implementor. Arguably, requirements on anyone other than the implementor in a spec are sort of useless. But we seem to do it, so...

Second, I note that a lot of specs seem to rely upon the WebIDL to define things it was never meant to do. Prose that explains the why of the interface is just as important as the structured WebIDL that defines the what of the interface. So, while this particular data is not important in that we are not losing the "why" of these parameters, let's try to keep it in mind.

Thanks for listening!

@rvm4 rvm4 merged commit 44dddcc into gh-pages Aug 29, 2016
@sideshowbarker sideshowbarker deleted the sideshowbarker/required-fields branch September 16, 2016 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants