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

Pin graphql core <3.1 #793

Merged

Conversation

chewselene
Copy link
Collaborator

3.1 contains a breaking change.

@bojanserafimov
Copy link
Collaborator

bojanserafimov commented Apr 8, 2020

Should also update setup.py.

Sad to see the semver https://semver.org/ principles broken. But probably this is on us since we rely on their internals

obi1kenobi
obi1kenobi previously approved these changes Apr 8, 2020
@obi1kenobi
Copy link
Contributor

3.1 is the Python equivalent of the Javascript v15 library.
3.0 is the Python equivalent of the Javascript v14 library.

The Javascript library seems to be following semver. I'm not sure what we were using is internal, so it may or may not be on us. We should maybe bring this up with the GraphQL-core maintainers, just to understand what our expectations should be around version bumps. In the meantime, I suggest we pin to minor versions instead of majors.

@obi1kenobi
Copy link
Contributor

@bojanserafimov can you take a look at the query_parameterizer.py lint issues? I think the version bump allowed mypy to do some deeper analysis and it's now unhappy.

@obi1kenobi
Copy link
Contributor

The helpers.py mypy issue seems like it might be an instance of python/mypy#5374 so let's # type: ignore it.

@bojanserafimov
Copy link
Collaborator

Sure. I'll send a diff for that file soon.

@bojanserafimov
Copy link
Collaborator

#794

@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #793 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #793   +/-   ##
=======================================
  Coverage   94.81%   94.81%           
=======================================
  Files         106      106           
  Lines        8241     8243    +2     
=======================================
+ Hits         7814     7816    +2     
  Misses        427      427           
Impacted Files Coverage Δ
graphql_compiler/compiler/helpers.py 93.59% <100.00%> (ø)
...l_compiler/query_pagination/query_parameterizer.py 97.16% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1af758d...0c3f234. Read the comment docs.

@bojanserafimov bojanserafimov merged commit 2ee68e0 into kensho-technologies:master Apr 8, 2020
@chewselene chewselene deleted the pin_graphql_core branch April 8, 2020 19:31
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