Contributing file updates, remove old development file #4246
Conversation
Co-authored-by: Ralf Handl <[email protected]>
There was a problem hiding this comment.
Excellent work, @lornajane , thank you so much!
I have various minor comments but no dealbreakers.
|
|
||
| ### Milestones | ||
|
|
||
| We use milestones in GitHub to plan what should be included in future releases. |
There was a problem hiding this comment.
Specifically, use the oldest-release-line milestone that is relevant if we are working on multiple releases at once.
| * Bring the future closer by making changes that are in line with future 3.x releases and the planned OpenAPI 4.x (Moonwalk) specification as the details of that become available. | ||
| * Make the specification document clearer or easier to understand. | ||
|
|
||
| A minor release is due when there are some meaningful features (including one or a small number of headline features). |
There was a problem hiding this comment.
I don't think we defined "headline feature" although it's fairly obvious. Just commenting in case we want to give it a definition- I'm fine with it as-is, though.
| 4. Use "OpenAPI Object" instead of "root". | ||
| 5. Fixed fields are monospaced. | ||
| 6. Field values are monospaced in JSON notation: `true`, `false`, `null`, `"header"` (with double-quotes around string values). | ||
| 7. A combination of fixed field name with example value uses JS notation: `in: "header"`, combining rules 5 and 6. |
There was a problem hiding this comment.
I think of this as YAML notation, although now that I read this I realize that JS might allow bar key names and require quoted values, so I guess this makes sense. But to folks like me who avoid JS like the plague, it's not the most obvious statement. I'm not going to be upset if it's left as-is, though.
| 5. Fixed fields are monospaced. | ||
| 6. Field values are monospaced in JSON notation: `true`, `false`, `null`, `"header"` (with double-quotes around string values). | ||
| 7. A combination of fixed field name with example value uses JS notation: `in: "header"`, combining rules 5 and 6. | ||
| 8. An exception to 5-7 is colloquial use, for example "values of type `array` or `object`" - "type" is not monospaced, so the monospaced values aren't enclosed in double quotes. |
There was a problem hiding this comment.
Can we add some note that "primitive" is not a formal type and should not be written primitive? We messed this up in the patch releases in a few places.
There was a problem hiding this comment.
I'm not sure how to provide an exhaustive list of words that shouldn't be formatted. The list above should help us just format the ones we do want, I think?
There was a problem hiding this comment.
Oh, I definitely wouldn't try to be exhaustive, and maybe there's a better way to handle it than a word list.
The problem we have in the new patch releases is that primitive looks like a literal type value impacting the style field, when really it's a category encompassing the literal values number, integer, string, boolean, and "null". That's confusing, although we do use "primitives" with an example using a string and in an example in another section.
Ideally, our style guide would help us avoid these category-vs-literal-value errors (although I don't know that we have any other such errors).
| * JSON Schema keywords -> "keyword" | ||
| * OpenAPI fixed fields -> "field" |
There was a problem hiding this comment.
Possibly too much of a corner case, but I do sometimes talk about Schema Object "fields" when talking about it in the context of OAS Objects as a whole, to avoid complicated "fields or keywords" phrasing. If we want a rule about this it might be worth an issue to discuss rather than holding up this PR.
Co-authored-by: Ralf Handl <[email protected]>
A pretty major overhaul of the contributing file. The main aims are to fill in gaps, capture informal conventions, and try to structure the information more for newcomers as well as project regulars.
A huge number of "we should write this down" issues are (at least partially) addressed in this update:
I think there's a lot here that probably isn't perfect, but let's try to merge when it's good enough to be considered an improvement on our current file, even if there are followup actions to take?