Document pluralization of API fields#2838
Conversation
|
This is almost verbatim from the doc, which was almost verbatim from the emails. I tried to make the lead-up to this section cleaner. There are a lot of small updates and edits that this doc needs, but I did not do most of that here. |
34a71cf to
1ab234d
Compare
| * which fields are required and which are not | ||
| * mutable fields do not become immutable | ||
| * valid values do not become invalid | ||
| * explicitly invalid values do not become valid |
There was a problem hiding this comment.
Might be confusing for a novice - how does valid values in previous bullet contrast to explicitly invalid, etc
There was a problem hiding this comment.
s/explicitly/previously/ (if you meant something else, more explanation is needed)
Note that one might expect this and the prior line to have a symmetry over upgrade/downgrade, but they don't?
There was a problem hiding this comment.
What I wanted to convey was the difference between "we expanded the regex to include dashes" from the case of open-ended enum fields, which is discussed elsewhere in the doc.
Suggestions?
| current value | ||
|
|
||
| Older clients that only know the singular field will continue to succeed and | ||
| produce the same results as before the change. Newer clients can use your |
There was a problem hiding this comment.
Probably worth noting that old clients break when trying to update the singular value of plural values are specified (because 0 has changed), since a lot people may not realize the implications
There was a problem hiding this comment.
Actually you have it below, disregard
| * which fields are required and which are not | ||
| * mutable fields do not become immutable | ||
| * valid values do not become invalid | ||
| * explicitly invalid values do not become valid |
There was a problem hiding this comment.
s/explicitly/previously/ (if you meant something else, more explanation is needed)
Note that one might expect this and the prior line to have a symmetry over upgrade/downgrade, but they don't?
| continue to function as they did previously, even when your change is utilized. | ||
| 5. Existing clients need not be aware of your change in order for them to | ||
| continue to function as they did previously, even when your change is in use. | ||
| 6. It must be possible to rollback to a previous version of API server that |
There was a problem hiding this comment.
How many versions are required to be roll-backable? I think just 1, but we should say that?
| 5. Existing clients need not be aware of your change in order for them to | ||
| continue to function as they did previously, even when your change is in use. | ||
| 6. It must be possible to rollback to a previous version of API server that | ||
| does not include your change and have no impact on API objects which do not use |
There was a problem hiding this comment.
...and what of objects that are using your change?
There was a problem hiding this comment.
I'd add words, but there's not much to say - a feature got rolled back. There's always the possibility of weird behavior in those cases. What would you say here?
There was a problem hiding this comment.
I think we owe it to people to give better guidance about what kinds of impact are acceptable. But we can fix that later, I think we don't actually have a great answer at the moment..
contributors/devel/api_changes.md
Outdated
| API, as above. | ||
|
|
||
| However, this is very challenging to do correctly. It often requires multiple | ||
| For some change, this can be challenging to do correctly. It may require multiple |
Based on https://docs.google.com/document/d/1Z8Vbo7RmV_Wvs4k8mluHQC2_Zs8cEwMJmHwWBf9BcaA/edit and email discussions. Resist the urge to clean up the whole doc - that will have to come later.
1ab234d to
e26bb61
Compare
|
Most comments addressed, also added words about de-duping plural. |
|
ping |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cblecker, lavalamp The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Based on
https://docs.google.com/document/d/1Z8Vbo7RmV_Wvs4k8mluHQC2_Zs8cEwMJmHwWBf9BcaA/edit
and email discussions.
Resist the urge to clean up the whole doc - that will have to come
later.