Skip to content

Conversation

@MikeRalphson
Copy link
Member

More nit-picking / doc cleanup ;)

  • Update the guidance text on key words to clarify only uppercase instances are to be interpreted as key words - this helps with ambguity over 'may' meaning 'might' and 'MAY' meaning 'it is permitted', 'not all ... must be' etc.
  • Update guidance to reference RFC8174
  • Uppercase key words where apparently needed
  • Make use of Required. / Required. / Required consistent => REQUIRED
  • Change one * to \* to fix an ambiguity over italic text and aid editing the spec in Vim's markdown syntax highlighting mode (other editors are probably available)
  • Erroneous whitespace has been removed only where it was on a line already being modified.

versions/3.0.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of linking the .txt version, I would recommend the HTML one.

Copy link
Contributor

@ePaul ePaul Jun 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template in RFC8174 says to actually mention both RFCs (and the BCP number):

  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
  NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
  "MAY", and "OPTIONAL" in this document are to be interpreted as
  described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
  appear in all capitals, as shown here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I hesitated on that and went with brevity over having the three links, but can amend.

versions/3.0.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if the word "REQUIRED" here actually conforms to RFC 2119, as it doesn't prescribe any behavior.
Maybe the initial paragraph referencing the RFCs and listing the keywords, or the first paragraph under "Schema" should get an additional sentence like this:

A sentence consisting of just the word REQUIRED in a field definition means that including the field is REQUIRED when sending the containing object.

versions/3.0.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to RFC8174, should the full language be:

  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
  NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
  "MAY", and "OPTIONAL" in this document are to be interpreted as
  described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
  appear in all capitals, as shown here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update.

versions/3.0.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're editing this line, would you want to also make it one sentence per line?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think elsewhere table cells are always written on one line?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table cells have to be one line. At least in the past that was the case.

@RobDolinMS
Copy link
Contributor

Thank you @MikeRalphson for this PR. I'm a fan of the clear use of CAPITALIZATION to indicate BCP14 words. I added two suggestions on specific lines.

@webron
Copy link
Member

webron commented Jun 9, 2017

I've yet had a chance to review the PR but something did catch my eye. The 'Required.' in field descriptions is our indication of a required field. It should not be capitalized.

@MikeRalphson
Copy link
Member Author

MikeRalphson commented Jun 9, 2017

The 'Required.' in field descriptions is our indication of a required field.

Well, yes, obviously. ;)

It should not be capitalized.

Happy to revert, but in what sense does Required. not "mean that the definition is an absolute requirement of the specification." ?

I specifically wanted to avoid increasing ambiguity by saying REQUIRED was only a key word when capitalised, then having it not capitalised where it does in fact mandate behaviour.

We could add extra wording e.g. as suggested by @ePaul .

@webron
Copy link
Member

webron commented Jun 9, 2017

Perhaps I shouldn't have jumped before reading the entire PR and conversation - I just didn't want extra work to be put into it for nothing. Unfortunately, I can't review it all right now.

@RobDolinMS
Copy link
Contributor

#TDC @RobDolinMS and @fehguy are both thumbs-up.

@RobDolinMS RobDolinMS merged commit 5471a18 into OAI:OpenAPI.next Jun 9, 2017
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.

5 participants