Skip to content
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

feat: rendering the syntax of different content types responses #665

Merged

Conversation

iifawzi
Copy link
Contributor

@iifawzi iifawzi commented Sep 11, 2022

Signed-off-by: iifawzi iifawzie@gmail.com
resolving #117

As per fastify/fastify#4264, swagger should support different content types responses.
Tests here will fail until the feature is merged at fastify, but I've hard-codded the fastify proposed changes and tested it.

Screen Shot 2022-09-11 at 8 44 46 PM

Screen Shot 2022-09-11 at 8 45 45 PM

Screen Shot 2022-09-11 at 8 45 52 PM

Checklist

Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi iifawzi changed the title feat: support rendering the syntax of the different content types responses feat: rendering the syntax of different content types responses Sep 11, 2022
Signed-off-by: iifawzi <iifawzie@gmail.com>
@Eomm
Copy link
Member

Eomm commented Sep 12, 2022

Tests here will fail until the feature is merged at fastify, but I've hard-codded the fastify proposed changes and tested it.

You can change&push temporary the package.json to use your fastify branch:

    "dependencies": {
        "fastify": "github:iifawzi/fastify#supporting-different-content-types"
    }

so we will check the green ci

lib/spec/openapi/utils.js Outdated Show resolved Hide resolved
test/spec/openapi/schema.js Show resolved Hide resolved
Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi iifawzi requested review from Eomm and removed request for climba03003 September 12, 2022 07:14
@iifawzi
Copy link
Contributor Author

iifawzi commented Sep 12, 2022

Tests here will fail until the feature is merged at fastify, but I've hard-codded the fastify proposed changes and tested it.

You can change&push temporary the package.json to use your fastify branch:

    "dependencies": {
        "fastify": "github:iifawzi/fastify#supporting-different-content-types"
    }

so we will check the green ci

Nice to know, committed.

lib/spec/openapi/utils.js Outdated Show resolved Hide resolved
Signed-off-by: iifawzi <iifawzie@gmail.com>
Signed-off-by: iifawzi <iifawzie@gmail.com>
Copy link

@zrosenbauer zrosenbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the misspelling please 😄

lib/spec/openapi/utils.js Outdated Show resolved Hide resolved
Signed-off-by: iifawzi <iifawzie@gmail.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
<br>
<br>
Different content types responses are supported by `@fastify/swagger` and `@fastify`.
Please use `content` for the response otherwise Fastify itself will fail to compile the schema:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Please use `content` for the response otherwise Fastify itself will fail to compile the schema:
Use `content` for the response otherwise Fastify itself will fail to compile the schema:

Nothing to please here ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please is used everywhere in the readme

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind removing it though

test/spec/openapi/schema.js Outdated Show resolved Hide resolved
@iifawzi
Copy link
Contributor Author

iifawzi commented Oct 14, 2022

since fastify/fastify#4264 is finally finished :D, CI should be green here, and then can be merged

Signed-off-by: iifawzi <iifawzie@gmail.com>
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
Signed-off-by: iifawzi <iifawzie@gmail.com>
@iifawzi
Copy link
Contributor Author

iifawzi commented Oct 15, 2022

committed @Uzlopak suggestions, and temporarily updated the packages to check ci.

@mcollina
Copy link
Member

Fastify v4.9.0 shipped.

Signed-off-by: iifawzi <iifawzie@gmail.com>
package.json Outdated Show resolved Hide resolved
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants