Merge result summary deprecation #5119
Conversation
Step before we depreate the merge summary result. This version will always return the summary fields and define the field as optional. It will enable users to switch to a version which will not break when the field is gone.
nopcoder
added
area/API
|
No linked issues found. Please add the corresponding issues in the pull request description. |
There was a problem hiding this comment.
Thanks!
I prefer to deprecate the fields rather than the message, because one day we might want to return some value. At the same time, please also mark all these fields as "deprecated", which issues warnings on Python I think, and add a scary "description".
THANKS!
api/swagger.yml
Outdated
| @@ -176,28 +176,26 @@ components: | |||
| items: | |||
| $ref: "#/components/schemas/Repository" | |||
|
|
|||
| MergeResultSummary: | |||
| description: "Deprecated: merge summary will be removed" | |||
There was a problem hiding this comment.
I would rather only deprecate all the fields and keep a MergeSummary, even empty. That will prevent future backwards-compatibility failures -- otherwise when we will want to add a response type, that's another breaking change :-(
| added: | ||
| type: integer |
There was a problem hiding this comment.
| added: | |
| type: integer | |
| added: | |
| type: integer | |
| deprecated: true | |
| description: Deprecated; inaccurate and will be removed. |
(Also on all other properties, of course.)
There was a problem hiding this comment.
@arielshaqed updated the PR with field level deprecation - note that I kept the summary as optional as I don't think we will set an empty structure if we keep it as required.
Merge API change and deprecation waring - step before we can remove the summary fields from merge result.
This version will always return the summary field and define the field as optional.
It will enable users to switch to a version which will not break when the field is gone.
Related to #3978