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

Adding deprecation notices for printer and parser, bumping bundled graphql version. #55

Merged
merged 9 commits into from
Feb 28, 2017

Conversation

jnwng
Copy link
Contributor

@jnwng jnwng commented Feb 24, 2017

per #54, this branch will contain the requisite changes for [email protected] (note that the base of this PR is the 1.3.0 branch, not master).

the bundled version of graphql/language/parser and graphql/language/printer is now v0.9.1, with deprecation notices to denote their removal in v2.

note: the printer and parser module have been changed from previously housing the compiled graphql modules, but now is just a proxy to the bundledParser and bundledPrinter so we can add in the deprecation warnings. please let me know if there's a better way to serve this notice.

@jnwng jnwng added this to the v1.3 milestone Feb 24, 2017
@jnwng jnwng assigned jnwng, stubailo and helfer and unassigned jnwng, stubailo and helfer Feb 24, 2017
@stubailo
Copy link
Contributor

Wow, GitHub really failed at showing the diff here.

Perhaps we can just warn at the top level? Like if someone does import { parse } from 'graphql-tag/parser and doesn't even call it, we can warn at import time.

@jnwng
Copy link
Contributor Author

jnwng commented Feb 25, 2017

@stubailo good call, i had done that earlier but was worried about double-warning, but turns out i was just overthinking it and i put the warnings in the module like you mentioned. they should get triggered on import of the module anywhere, not on execution of the function (which could happen potentially many times)

@stubailo
Copy link
Contributor

This looks great!

@helfer
Copy link
Contributor

helfer commented Feb 28, 2017

@jnwng alright, looks good. I'll merge this into 1.3 to release, and I'll let you merge it into master from 1.3 to release 2.0 later.

@helfer helfer merged commit fc55c2a into apollographql:1.3.0 Feb 28, 2017
@jnwng jnwng deleted the jw/v1.3 branch February 28, 2017 23:02
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