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(descriptors): add support for descriptors #99

Merged
merged 2 commits into from
Jun 16, 2017

Conversation

jamiter
Copy link
Contributor

@jamiter jamiter commented Jun 15, 2017

Connects to #81

This will add support for descriptors in schema files:

# This is a type descriptor
type Foo {
  # This is a field descriptor
  bar: String
  baz: Int
}

@stubailo, could you have a look at this? Or maybe @jnwng. Thanks!

@dyst5422
Copy link

Looks good to me.

test.js Outdated
}
`;

assert.equal(ast.definitions[0].description, 'This is a descriptor');

Choose a reason for hiding this comment

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

Probably want to add an assertion on the second descriptor too.

@jamiter
Copy link
Contributor Author

jamiter commented Jun 16, 2017

@dyst5422, thanks for the feedback! I added more tests to cover type and field descriptors.

@jnwng jnwng merged commit 78ed906 into apollographql:master Jun 16, 2017
@jnwng
Copy link
Contributor

jnwng commented Jun 16, 2017

thank you @jamiter!

@jnwng jnwng mentioned this pull request Jun 16, 2017
@jamiter jamiter deleted the feature/descriptors branch June 16, 2017 18:50
@jamiter
Copy link
Contributor Author

jamiter commented Jun 16, 2017

@jnwng, you're welcome, thanks for merging! I see I didn't update the changelog though. Is that something you like to do yourself? Seems 2.3.0 is also missing. Not sure what's in there.

@jnwng
Copy link
Contributor

jnwng commented Jun 16, 2017 via email

@dyst5422
Copy link

Will you be tagging a release on this? Would love to pull it down and start seeing my descriptors.

@jnwng
Copy link
Contributor

jnwng commented Jun 17, 2017

@dyst5422 this is in graphql-tag@2.4 on npm, but i did just notice that the tags i'm creating locally haven't been pushed. will update that when i get a chance.

@jnwng
Copy link
Contributor

jnwng commented Jun 17, 2017

https://github.com/apollographql/graphql-tag/releases/tag/v2.4.0 happy hacking!

@helfer
Copy link
Contributor

helfer commented Jun 17, 2017

Just an idea: we could add a postpublish script that runs git push --tags. I've done similar things in our other repos, which has helped me not forget to do it.

@jnwng
Copy link
Contributor

jnwng commented Jun 22, 2017

@jamiter just an FYI — there's been a couple of issues #106 #104 that i believe are associated with this particular part of the graphql-js code. i've temporarily reverted this change in 2.4.1 to buy us some time to investigate, you should still be able to use 2.4.0.

@jamiter
Copy link
Contributor Author

jamiter commented Jun 22, 2017

Thanks for the heads up. Could it be an issue with graphql/utilities/buildASTSchema? Maybe it's not availble in some versions of graphql...

jnwng added a commit that referenced this pull request Jun 27, 2017
jnwng added a commit that referenced this pull request Jun 27, 2017
jamiter pushed a commit to jamiter/graphql-tag that referenced this pull request Dec 18, 2017
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.

4 participants