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

Remove transformed queries #1859

Merged

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Dec 14, 2019

Now that #1841 is merged, this PR can be merged.

This PR:

@martinbonnin
Copy link
Contributor Author

Instead of using transformedQueriesDir with a flat dir of all transformedQueries, you would now have to use operationOutputFile that is a json that maps operationIds to transformedQueries.

You can use OperationOutput(file: File) to create pojos from the json file.

@DSteve595 is it ok for you ?

@DSteve595
Copy link
Contributor

Sounds great. Thank you all for your hard work.

Copy link
Contributor

@tasomaniac tasomaniac left a comment

Choose a reason for hiding this comment

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

I have very small comments. Thanks for taking care of this. It definitely keeps the API surface cleaner.

Copy link
Contributor

@tasomaniac tasomaniac left a comment

Choose a reason for hiding this comment

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

Looks good. Btw, is this a breaking change?

@martinbonnin
Copy link
Contributor Author

I guess it's a breaking change to whoever is relying on transformed queries. It doesn't change anything to the runtime and/or codegen. Overall, I would say users of transformed queries are advanced enough to migrate to operation output but it's definitely worth a mention in the changelog.

@tasomaniac
Copy link
Contributor

tasomaniac commented Dec 25, 2019

I guess mention in the changelog would be enough. Their build configuration will fail to find the old property. It's a really simple change to fix it.

@tasomaniac tasomaniac merged commit 8d2d478 into apollographql:master Dec 25, 2019
@martinbonnin martinbonnin mentioned this pull request Jan 8, 2020
8 tasks
@martinbonnin martinbonnin deleted the remove-transformed-queries branch May 21, 2020 13:10
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