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

graphql 3.0 and graphene 3.0 final rebase #951

Merged
merged 13 commits into from May 9, 2020
Merged

graphql 3.0 and graphene 3.0 final rebase #951

merged 13 commits into from May 9, 2020

Conversation

ganwell
Copy link
Contributor

@ganwell ganwell commented Apr 29, 2020

I rebased almost all change to their original author: form #905 and #774.

@ganwell ganwell requested review from jkimbo and ulgens April 29, 2020 13:50
@ganwell ganwell changed the title WIP: @ulgens plus fixing ResolveInfo v3: @ulgens #905 plus fixing ResolveInfo Apr 29, 2020
@ganwell ganwell marked this pull request as ready for review April 29, 2020 13:58
@ganwell ganwell changed the title v3: @ulgens #905 plus fixing ResolveInfo v3: #905 plus fixing ResolveInfo Apr 29, 2020
Copy link

@winged winged 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, apart from some minor issues.

Maybe also update the commit from @ulgens to contain all the notes from the squashed commits

graphene_django/converter.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@ganwell
Copy link
Contributor Author

ganwell commented May 1, 2020

@jkimbo I reviewed the PR with my college @winged.

  • I will fix setup.py

  • And move the relevant commit messages from all the v3-attempts into the main-commit

Then we think, it can be merged into v3. Then I try v3 with caluma.

Copy link
Member

@jkimbo jkimbo left a comment

Choose a reason for hiding this comment

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

This is looking good thanks @ganwell ! 1 minor suggestion.

README.rst Outdated Show resolved Hide resolved
@jkimbo
Copy link
Member

jkimbo commented May 1, 2020

@ganwell I've just merged the github actions migration into v3 so you'll have to rebase this change on top of that (sorry)

ganwell and others added 13 commits May 4, 2020 12:05
* Update mentions of django 1.11 and 2.0
Fields starting with __ are reserved for introspection
Connection resolvers have access to pagination arguments.
Some of the tests show the problem described here: graphql-python/graphql-core#61
It was only needed for Python 2.7
…ray_slice

connection_from_list_slice:
> Deprecated alias for connection_from_array_slice. We're now using the
JavaScript terminology in Python as well, since list is too narrow a
type and there is no other really appropriate type name.

https://github.com/graphql-python/graphql-relay-py/blob/v3.0.0/src/graphql_relay/connection/arrayconnection.py#L54
* names must be strings

* update to expected schemas to new format

* Call get() on django promises to get the actual value
@ganwell ganwell changed the title v3: #905 plus fixing ResolveInfo graphql 3.0 and graphene 3.0 final rebase May 4, 2020
@ganwell
Copy link
Contributor Author

ganwell commented May 4, 2020

@jkimbo this is hopfully the final rebase. I rebased all original commits from @ktosiek and @ulgens in to this PR. The diff to the version that was reviewed is trivial: https://git.io/Jfsh3

Copy link

@winged winged left a comment

Choose a reason for hiding this comment

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

I like :) Nice work on the rebase / re-include of the contributors as well!

Copy link
Member

@jkimbo jkimbo 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 @ganwell . Thanks for all your work on this!

@jkimbo jkimbo merged commit 10d22de into graphql-python:v3 May 9, 2020
This was referenced May 9, 2020
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.

None yet

5 participants