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

WIP: GraphQL 4 compat #32

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

unclecheese
Copy link

No description provided.

@tractorcow
Copy link
Collaborator

tractorcow commented Dec 8, 2020

Assuming that graphql 4.x is the full rewrite, this course of action is correct.

master -> aliased as 3.0 can be the new upgrade.
2.0 -> old graphql 3 compat module.

@unclecheese there is another outstanding issue with the upstream jwt dependency. Can you also please upgrade this to ^4 in the same upgrade? (otherwise we will have TWO major version releases for each).

See #31 for my failed attempt to fix the deprecations for JWT 3.4. The correct fix is to release a new major version and just use JWT 4.x directly.

My other suggestion for 2.0 branch is to lock the JWT dependency to ~3.3.0

@tractorcow
Copy link
Collaborator

Oh, and I can help with this work too, since I'll need this for my next project :)

@unclecheese
Copy link
Author

I have no plans to make it BC. I'll do this to the master branch and we can tag 3.0 with a strict graphql 4 dependency.

It is possible to do it BC, but it's tedious and I think it's only worth it for core modules.

@Firesphere
Copy link
Owner

Please ignore the CircleCI failures. This is due to an issue with upgrading MariaDB on CircleCI and not related to the module.

I haven't had time to fix it, but if you can show the tests are green on your end, I don't mind

@Firesphere
Copy link
Owner

Handing over to @tractorcow for this one. I don't have the time or energy :)

@unclecheese
Copy link
Author

It kind of works. Seems to be an issue in the unit tests where Members are expected to come back null in the auth, but they're not.

@Firesphere
Copy link
Owner

I seriously have trouble keeping up with everything. I am more than happy to keep this module alive, but I need someone else to take the wheel. I have 200 hours of work on my modules a day. And sadly, a day only has 24 hours. Excluding time I need to rest.

I nominate @tractorcow to be the new primary developer/maintainer, and fully trust him to add other maintainers.

@guci0
Copy link

guci0 commented Jan 26, 2021

It kind of works.

Some little example of how to implement this to make it work. I had an approach once, but I couldn't make it ... Certainly not one person would help. Thanks in advance!

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