-
Notifications
You must be signed in to change notification settings - Fork 9.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVIEW ONLY] Add written-word drafts since a PR is a good way to review #4095
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Only minor nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
Will the 3.0.4 releases be done in a separate PR or will they be added to this one?
release-notes-3.1.1.md
Outdated
- examples and additional documentation moved to https://learn.openapis.org | ||
- the HTML rendering of the specification at https://spec.openapis.org is now considered the authoritative version (formerly it was the Markdown source on GitHub) | ||
|
||
OpenAPI description writers should mark their OpenAPI descriptions with the version of OpenAPI specification they used to write their specification, updating where possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Can this be explained more? (note that this is more of a request to description document authors than changes to 3.1.1, so perhaps break it out of this list?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! Some wordsmithing suggestions to consider.
Co-authored-by: Darrel <darrmi@microsoft.com>
7c3faf6
Co-authored-by: Marsh Gardiner <marsh.gardiner@gmail.com>
including the two I missed before Co-authored-by: Marsh Gardiner <marsh.gardiner@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lornajane Looks good to me. Thanks.
@mikekistler The release notes are almost identical for both versions, I ended up with nothing to add for 3.0.4, and just a few lines that are 3.1.1-only, these are clearly marked. We'll need to edit/remote them for each version accordingly when creating the releases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work- I have some suggestions but I don't think there's anything here that I'd consider a blocker.
I feel like we should probably explain tooling expectations around the patch number in the openapi
field more clearly somewhere, but I don't know that this is the place to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many of @handrews' suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still looks good to me after the various last-minute changes. I marked some conversations resolved as they either involved a response to me, or mentioned Marsh and he put a thumbs-up on the mention.
I think the wording on OpenAPI Description and lowercase-document are sufficient here, and we don't need ot get into the level of "document" vs "OpenAPI Document" in this announcement. The usage of lowercase-document matches how we still use that word in the final updates.
In preparation for the release, please review (BUT DO NOT MERGE) the proposed wording for release notes and a blog post. I couldn't think of a better place to put words that we could all review, comment on and edit so I hope this makes sense. Pull request contains:
[3.1.1]
are for that version only, I'll take the whole line out for the 3.0.4 release notes, and remove the indicator for the 3.1.1 ones - it just seemed easier than providing two almost-identical documents for you to reviewWith a big thankyou to @handrews for mining the epic diff to produce a huge starting point of information that I've tried to usefully summarise without eliminating all the detail!
The goal here is to check for mistakes and showstoppers; let's try not to polish forever. This one is NOT an authoritative document that must never be updated!