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

Fix migration warnings in scio-core #9

Merged
merged 3 commits into from
Apr 15, 2021

Conversation

MaximeKjaer
Copy link

It seems like it's necessary to not be in migration mode in order to use the new Scala 3 given keyword. In order to be able to use this, I've fixed the migration warnings in scio-core, and have removed the migration mode.

I used the -rewrite option to help me fix some warnings. For the do-while, I preferred to rewrite it using a var rather than the recommended style, which I somewhat illegible...

@MaximeKjaer
Copy link
Author

Oh no... Seems like some dependencies are using bintray, which is shutting down. Seems like dependencies have been somewhat updated on upstream / main, so we may need to do a rebase to fix this. I'll take care of it.

Copy link

@vincenzobaz vincenzobaz left a comment

Choose a reason for hiding this comment

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

Are the private[scio] specific enough? For example could you replace with a subpackage such as private[coders] or private[schema]?

As long as it is in scio, we should be good to go. However using a more specific limiter is closer to the original private

@MaximeKjaer
Copy link
Author

Thank you, this is a good point. Where possible, I've made the private scope of constructors more specific.

@MaximeKjaer
Copy link
Author

If tests pass, I suggest we squash and merge.

@MaximeKjaer MaximeKjaer merged commit 392d34c into scala3-main Apr 15, 2021
@MaximeKjaer MaximeKjaer deleted the fix-scio-core-migration-warnings branch April 15, 2021 11:20
MaximeKjaer added a commit that referenced this pull request Apr 19, 2021
MaximeKjaer added a commit that referenced this pull request May 5, 2021
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.

2 participants