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

[TS] Added outputGlobalTypes option to specify path for global types #546

Merged
merged 4 commits into from
Aug 31, 2018

Conversation

danilobuerger
Copy link
Contributor

No description provided.

@ghost ghost added the 🎉 feature New addition or enhancement to existing solutions label Aug 12, 2018
@williamboman
Copy link

This is great! I think it would also be nice to be able to set the filename (globalTypes.ts).

@danilobuerger
Copy link
Contributor Author

danilobuerger commented Aug 13, 2018

@williamboman Thats exactly what this option is for, the path includes the filename. For example: --outputGlobalTypes=__foo__/bar.ts

@williamboman
Copy link

Oh shit, my bad!

@zionts
Copy link
Contributor

zionts commented Aug 24, 2018

@danilobuerger thanks for submitting this! Could you update https://github.com/apollographql/apollo-cli/blob/master/CHANGELOG.md with what this PR is doing?

@@ -95,6 +95,10 @@ export default class Generate extends Command {
description:
'By default, TypeScript/Flow will put each generated file in a directory next to its source file using the value of the "output" as the directory name. Set "outputFlat" to put all generated files in the directory relative to the current working directory defined by "output".'
}),
outputGlobalTypes: flags.string({
Copy link
Contributor

Choose a reason for hiding this comment

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

Not doing a full review, but I think that outputGlobalTypes is a strange name (and reads like a boolean) for a string that is meant to specify a file location to write global type information to. Perhaps something like globalTypesFilePath?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention was that it is aligned with the other output options (output and outputFlat) for easier grouping if they all begin with output.... But I really don't care what the param is named :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree outputGlobalTypes feels a bit weird as a flag name. Maybe globalTypesFile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martijnwalraven I amended the commit

@danilobuerger
Copy link
Contributor Author

@zionts In this repo the Changelog is done on Publish. See https://github.com/apollographql/apollo-cli/commits/master/CHANGELOG.md not during PRs.

abernix added a commit that referenced this pull request Aug 31, 2018
abernix added a commit that referenced this pull request Aug 31, 2018
@martijnwalraven martijnwalraven merged commit 03e2824 into apollographql:master Aug 31, 2018
@rahuljaindream11
Copy link

Will this key work for swift target

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants