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

fix(client-cli): Properly interpret nullable prop #1938

Conversation

rozzilla
Copy link
Contributor

With this change, when an Open API schema contains a nullable: true property, it's properly handled and the generated type will contain | null;

Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.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

@rozzilla
Copy link
Contributor Author

I've seen a coverage failure, but locally I get 100% 🤔
image

@mcollina
Copy link
Member

Slap a /* c8 ignore next 2 */ in there.

Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com>
Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com>
Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com>
@rozzilla
Copy link
Contributor Author

@mcollina not sure it's related to coverage 🤔
it's failing other packages + looking at the failure it seems some tests are cancelled, like in this case + the same thing seems to already happen on master.

So I'm not sure it's related to this PR to be honest

@mcollina mcollina merged commit 0e56c71 into platformatic:main Dec 18, 2023
68 of 85 checks passed
@@ -400,6 +400,7 @@ async function plugin (app, opts) {
client = await buildOpenAPIClient(opts, app.openTelemetry)
} else if (opts.type === 'graphql') {
if (!opts.url.endsWith('/graphql')) {
/* c8 ignore next 2 */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI @mcollina probably this can be removed in another PR 😉

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.

None yet

2 participants