Skip to content

Upgrade kotlin and kotlin-coroutines to versions 1.3.72 and 1.3.9 #432

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

Conversation

brannstrom
Copy link

Fixes #378

Checklist

  • Pull requests follows the contribution guide
  • New or modified functionality is covered by tests

Description

Replaced deprecated CoroutineScope.publish(...) with top-level publish(...) method in SchemaParserOptions(See: https://github.com/Kotlin/kotlinx.coroutines/blob/master/reactive/kotlinx-coroutines-reactive/src/Publish.kt). No new tests were added but existing ones looked sufficient to me. I noticed that @oliemansm didn't upgrade the kotlin-coroutines lib in the "upgrade-dependencies" branch (See: commit 4543ccd) so there might be a reason to not do this update that I am not aware of?!

The CoroutineScope.publish(...) method was deprecated in favour of the global publish(...) method. See Publish.kt in kotlinx-coroutines-reactive lib.
Copy link
Member

@vojtapol vojtapol left a comment

Choose a reason for hiding this comment

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

This looks good. I would be happy to merge once we get 2 more approvals from the community. Especially, from those that actively use coroutines with `graphql-java-tools.

@brannstrom brannstrom requested a review from vojtapol September 10, 2020 07:44
@rrva
Copy link
Contributor

rrva commented Sep 11, 2020

Looks good to my eyes. Had a quick test run with it and passes our integration tests.

@brannstrom
Copy link
Author

@vojtapol Hi, is it possible to get any progress with this PR?

Copy link
Member

@vojtapol vojtapol left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution. Note that it may take some time for a new version of the library to be released.

@vojtapol vojtapol merged commit d780322 into graphql-java-kickstart:master Sep 28, 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.

Upgrade kotlin coroutines to 1.3.5 blocked by deprecated publish
3 participants