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: intrinsic type params are not Partial<T> #819

Merged
merged 2 commits into from
Sep 15, 2021
Merged

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Sep 15, 2021

codegen now generates affected body parameters as string instead of Partial<string>.

Typically body parameters are structures rather than simple (aka intrinsic) types like a string. While TypeScript correctly handles Partial<string> this is confusing to the code reader, so this tweaks the generator to remove Partial<*> wrapping of the type name for the body parameter on simple types.

This PR uses Looker 21.14 specifications for the updated SDK code that got generated.

codegen generates affected `body` parameters as `string` instead of `Partial<string>`.
@github-actions
Copy link
Contributor

Codegen Tests

  1 files    6 suites   56s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 870d12b.

@github-actions
Copy link
Contributor

Typescript Tests

    6 files    75 suites   3m 47s ⏱️
164 tests 160 ✔️   4 💤 0 ❌
546 runs  534 ✔️ 12 💤 0 ❌

Results for commit 870d12b.

@github-actions
Copy link
Contributor

Codegen Tests

  1 files    6 suites   58s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit c4e39fb.

@github-actions
Copy link
Contributor

Typescript Tests

    6 files    75 suites   4m 13s ⏱️
164 tests 160 ✔️   4 💤 0 ❌
546 runs  534 ✔️ 12 💤 0 ❌

Results for commit c4e39fb.

Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

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

LGTM

@jkaster jkaster merged commit 4b31490 into main Sep 15, 2021
@jkaster jkaster deleted the jk/impartial_simple_type branch September 15, 2021 22:01
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

2 participants