Skip to content

Migrate source_gen to null-safety #515

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

Closed
wants to merge 3 commits into from

Conversation

simolus3
Copy link
Contributor

@simolus3 simolus3 commented Mar 12, 2021

Migrates source_gen, tests and the example.

Still blocked by

Closes #493

@jakemac53
Copy link
Contributor

It would be good to have a version that allows build version 2.0.0 first (as well as 1.x), and then do this migration. That should allow people to migrate a bit more incrementally.

@jakemac53
Copy link
Contributor

Agreed, should I open another PR for that or is someone working on that already?

Nobody is actively working on it right now afaik, either way :). #516 is the issue which nobody has grabbed yet.

@simolus3
Copy link
Contributor Author

The only remaining dependency override is in example_usage, so this should be ready for review now :)

@simolus3 simolus3 marked this pull request as ready for review March 16, 2021 21:04
@kevmoo
Copy link
Member

kevmoo commented Mar 16, 2021

I think we duplicated work - #520

@simolus3
Copy link
Contributor Author

I'm ok with closing this PR then, but this isn't great. I spent a non-trivial amount of time on this and announced that I was going to work on this in the tracking issue quite some time ago. Very respectfully, I don't see how I duplicated work here.
Whatever, I'm glad we can get this package migrated.

@simolus3 simolus3 closed this Mar 16, 2021
@kevmoo
Copy link
Member

kevmoo commented Mar 16, 2021

I'm ok with closing this PR then, but this isn't great. I spent a non-trivial amount of time on this and announced that I was going to work on this in the tracking issue quite some time ago. Very respectfully, I don't see how I duplicated work here.
Whatever, I'm glad we can get this package migrated.

You're welcome to be upset here. I'm sorry. There are some non-trivial code paths I wanted to double check.

...and honestly I didn't see your PR before I started.

I'd love your eyes on my review, if you're willing.

Regardless, my sincere apologies. Thanks for your help here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for nullsafety
3 participants