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

feat: added -n | --nostreams to code generator options #549

Merged
merged 2 commits into from
Apr 1, 2021
Merged

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Apr 1, 2021

  • for SDKs that support streaming, this skips generating the streams file. It does not remove any existing streams files, or change any other generated files
  • expanded the -h display
  • invalid gen options are rejected with an error
  • recognize 'all' as a valid language switch but it can still be overridden if other language specifiers are used, so it's undocumented

- for SDKs that support streaming, this skips generating the `streams` file. It does *not* remove any existing streams files, or change any other generated files
- also expanded the `-h` display
- also support `all` as a language switch
Copy link
Contributor

@joeldodge79 joeldodge79 left a comment

Choose a reason for hiding this comment

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

one question but not blocking an LGTM

langs.push(gen.language)
}
})
if (values[0] !== 'all') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little lost about how 'all' comes into play (both here and L103)

Copy link
Contributor Author

@jkaster jkaster Apr 1, 2021

Choose a reason for hiding this comment

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

it's possible someone may do, for example:

yarn -n all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could probably remove the premature optimization bit of 103, but it's such a corner case ...

@jkaster jkaster merged commit 6ead15a into main Apr 1, 2021
@jkaster jkaster deleted the jk/no_stream branch April 1, 2021 23:45
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.

Seems okay but I do have a question about the help text

yarn gen

# Generates Typescript and Python SDKs
yarn gen ts,py
Copy link
Collaborator

Choose a reason for hiding this comment

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

so does it accept the file extension? I've been using yarn gen Typescript

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup. extension, label, or language name matches. Added that a little while ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because I'm lazy

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.

3 participants