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

Prepare for NNBD #99

Closed
rxlabz opened this issue Oct 8, 2020 · 11 comments
Closed

Prepare for NNBD #99

rxlabz opened this issue Oct 8, 2020 · 11 comments

Comments

@rxlabz
Copy link
Contributor

rxlabz commented Oct 8, 2020

For now the package has almost 200 errors with NNDB activated cf. https://dart.dev/null-safety

@rxlabz rxlabz added this to To do in kanban via automation Oct 8, 2020
@bruno-garcia
Copy link
Member

@rxlabz would be great to add null safety as early as possible. Since retrofitting it is much harder than adding code while having it turned on. @marandaneto ?

@marandaneto
Copy link
Contributor

marandaneto commented Oct 9, 2020

yep I'd love to turn it on as well but we'd need to see how much work is to fix all the 200 errors.
@rxlabz would be possible to do a small investigation to understand how much work would we need to tackle all those null-safety issues?

@rxlabz
Copy link
Contributor Author

rxlabz commented Oct 9, 2020

The number is high because of the json encoding/decoding, it makes a lot of potentially null values. Should not be very long/hard to fix. Anyway, I think the Dart team talked about releasing a tool to help the migration. Next week I'll check if something is already available.

@bruno-garcia
Copy link
Member

@rxlabz worth noting that from Sentry's perspective {} is a valid event. So literally everything is optional. Not everything is documented optional, because some things wouldn't make sense like you send a frame line number but no function name. And other combination of data.

@rxlabz
Copy link
Contributor Author

rxlabz commented Oct 9, 2020

Thanks, I'll keep that in mind !

@rxlabz rxlabz self-assigned this Oct 12, 2020
@rxlabz rxlabz moved this from To do to In progress in kanban Oct 12, 2020
@marandaneto marandaneto moved this from In progress to To do in kanban Oct 13, 2020
@marandaneto
Copy link
Contributor

NNBD is not Flutter ready and we are blocked by it, let's revisit this at a later time.

@marandaneto marandaneto removed this from To do in kanban Dec 4, 2020
@MisterJimson
Copy link

Flutter now recommends migration for packages and has a migration tool for assistance: https://dart.dev/null-safety/migration-guide

@marandaneto
Copy link
Contributor

marandaneto commented Dec 17, 2020

@MisterJimson true, I've been watching this tool, the thing is, supporting null safety right now would require to bump the min. Dart and Flutter SDK versions and not everyone is comfortable using beta versions right.

See: https://develop.sentry.dev/sdk/philosophy/#compatibility-is-king

Ideally, we'd do that using a branch, so we could publish a Prerelease with the null-safety feature, but still supporting older versions.
I'd start this only when our dependencies are null-safety ready though, see #247

@kuhnroyal
Copy link
Contributor

A -nullsafety Prerelease would be great. This is sort of a hen/egg problem at the moment.

@MisterJimson
Copy link

MisterJimson commented Dec 17, 2020

Absolutely I would expect null safety as a pre release and not in the main line for a while, just as the migration docs suggest.

@bruno-garcia
Copy link
Member

We'll implement this as part of #247
Tracking dependencies and progress on that issue.

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

No branches or pull requests

5 participants