Skip to content

fix: Fix optional model properties BNCH-20284 #35

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

Merged
merged 2 commits into from
Jan 27, 2021

Conversation

forest-benchling
Copy link

@forest-benchling forest-benchling commented Jan 21, 2021

Fixes the case when a model property is nullable, and adds E2E test coverage for this case.

I'm going to upstream this, but was planning to wait until openapi-generators#297 is merged (and didn't want to lump it into that PR to avoid scope creep).

Going to test by pulling this change into the benchling-sdk and confirming that integration tests pass (in particular for the Container and Location services).

Upstream PR: openapi-generators#315

@forest-benchling forest-benchling changed the title Fix model properties fix: Fix optional model properties BNCH-20284 Jan 21, 2021
@forest-benchling forest-benchling marked this pull request as ready for review January 21, 2021 16:10
Copy link

@bowenwr bowenwr left a comment

Choose a reason for hiding this comment

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

LGTM. If you're regenerating the client to test, you'll want to merge this into the benchling-sdk-m1-fixes branch on openapi-specs and integration test the whole client to make sure there are no regressions. Let me know if you need help with that.

@forest-benchling forest-benchling changed the base branch from benchling-issue-285 to main-v.0.7.3 January 27, 2021 14:31
@forest-benchling
Copy link
Author

Ugh, messed up the merge

@forest-benchling forest-benchling merged commit f49d0bc into main-v.0.7.3 Jan 27, 2021
@forest-benchling forest-benchling deleted the forest-model-property branch January 27, 2021 14:35
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.

2 participants