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 warning in generated server code #209

Merged
merged 5 commits into from
Aug 17, 2023

Conversation

arthurcro
Copy link
Contributor

@arthurcro arthurcro commented Aug 17, 2023

Motivation

Fixes #190.

Modifications

We fix the issue buy only generating this initializer if there's at least one operation. Otherwise keep the method empty.

Result

The warning is fixed since the server variable declaration is not added to the registerHandlers body when it's not used.

Test Plan

We add a test in SnippetBasedReferenceTests.swift called testServerRegisterHandlers_noOperations asserting the server variable delcaration is not added to the registerHandlers body.

@arthurcro
Copy link
Contributor Author

Hey @czechboy0! I'm not sure where to add a proper test for this, do you have any idea how I could write a good test for this scenario? 🙂

@czechboy0
Copy link
Contributor

Sure - using a snippet test would be ideal. But I'll have to create a new util function for it, let me push it directly into your branch, will let you know when it's ready.

@czechboy0
Copy link
Contributor

@arthurcro here you go: 2faf804

I created a test called testServerRegisterHandlers_oneOperation. I'll leave it as an exercise to you to create testServerRegisterHandlers_noOperations 🙂

@arthurcro
Copy link
Contributor Author

That's aweomse @czechboy0, thank you so much! I'll add the missing test as soon as possible and mark the PR as ready! 🙂

@arthurcro arthurcro force-pushed the fix-generated-server-warning branch from d95b7ee to 49b3115 Compare August 17, 2023 20:04
@arthurcro arthurcro marked this pull request as ready for review August 17, 2023 20:07
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@czechboy0 czechboy0 enabled auto-merge (squash) August 17, 2023 20:13
@czechboy0 czechboy0 merged commit d29fd40 into apple:main Aug 17, 2023
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix warning in generated server when no operations are included in the OpenAPI doc
2 participants