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

do not type hint mixed parameter types for request bodies in endpoint constructor and clients operations method #158

Merged
merged 3 commits into from
Oct 22, 2019

Conversation

lordrhodos
Copy link

see #157

Resolved parameter types for response bodies of type mixed are used as type hints in endpoint constructor and related client operation method signatures.

This PR fixes this by using the default parameter type null for these cases.

@Korbeil
Copy link
Member

Korbeil commented Oct 21, 2019

Could you add a related fixture in tests please ?

@lordrhodos
Copy link
Author

lordrhodos commented Oct 21, 2019

@Korbeil I am not familiar with the test cases for this lib and had just a quick check. The approach seems pretty straight forward, but I am not sure if I should extend an existing test case or create a new one. Which one do you prefer?

@coveralls
Copy link

coveralls commented Oct 21, 2019

Pull Request Test Coverage Report for Build 577

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 87.669%

Totals Coverage Status
Change from base Build 576: 0.06%
Covered Lines: 3377
Relevant Lines: 3852

💛 - Coveralls

@Korbeil
Copy link
Member

Korbeil commented Oct 21, 2019

You just have to go to src/OpenApi/Tests/fixtures then add a directory with the name you want and add a schema matching your case with expected generated Client

@lordrhodos
Copy link
Author

@Korbeil ok, will add a new test fixture, thx.

@Korbeil
Copy link
Member

Korbeil commented Oct 22, 2019

I don't see a Normalizer for your FooBar object, isn't that a problem ? We should have a Normalizer and a Model class here I think 🤔
There is probably more to fix for your issue.

@lordrhodos
Copy link
Author

@Korbeil I agree. For our usage at the moment it is not a problem as it reuses the same model in another endpoint so for that context it seems to be fine, but I agree that the root of the problem may be the correct handling of the anOf operator. I will have to dig deeper.

@Korbeil
Copy link
Member

Korbeil commented Oct 22, 2019

Okok, let's forget that for this PR so.
Could you fix the CS issue ?

@lordrhodos
Copy link
Author

yes, rebased and fixed

@Korbeil Korbeil merged commit 958af72 into janephp:master Oct 22, 2019
@Korbeil
Copy link
Member

Korbeil commented Oct 22, 2019

Thanks for your contribution 😄

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

3 participants