-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[NO TESTS NEEDED] swagger: add missing schema properties #9117
[NO TESTS NEEDED] swagger: add missing schema properties #9117
Conversation
cc @baude |
thanks for contribute, and your commit messages should change to include |
Fixes: #8577 |
Do we need this change in all of the register_*go files? |
ee1f811
to
e6b029c
Compare
@rhatdan I think this covers all needed cases. Do you have some specific section in mind that still should be patched? Is this good to merge? |
Could you repush to see if we get by the smoke test. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, tmds The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I think thew NO_TESTS_NEEDED needs to be in the body of your commit message. Speaking of which, can you provide a commit message? |
LGTM other than my comments about commit messages |
@tmds Please let me know what tool you are using, or why you need these changes? I'm curious if we need to add that property to all our responses. |
I'm using a .NET specific tool: https://github.com/RicoSuter/NSwag Without the
The other responses already include it, for example: BadParamError:
description: Bad parameter in request
schema:
properties:
cause:
description: API root cause formatted for automated parsing
example: API root cause
type: string
x-go-name: Because
message:
description: human error message, formatted for a human to read
example: human error message
type: string
x-go-name: Message
response:
description: http response code
format: int64
type: integer
x-go-name: ResponseCode
type: object
|
Is this good to merge? |
/lgtm |
Wait, CI is red |
@tmds I think you need a force-push to make the [NO TESTS NEEDED] label work - I recommend changing something trivial (punctuation in a description, maybe) and force-pushing an amended commit |
LGTM |
@tmds Could you rebase your PR and repush, so that the tests will notice the [NO TESTS NEEDED] flag and we can get this in. |
e6b029c
to
bbc86bb
Compare
I've rebased. |
NO_TESTS_NEEDED Signed-off-by: Tom Deseyn <[email protected]>
bbc86bb
to
a2e1b3e
Compare
/lgtm |
@jwhonce ptal.