-
Notifications
You must be signed in to change notification settings - Fork 42
Conversation
devoncarew
commented
Nov 7, 2020
- update to support 2.12.0 and null safety
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I'm just figuring this out as I go... Hope I'm not just muddying the waters with all the questions!
example/example.dart
Outdated
|
||
Analytics getAnalytics() { | ||
Analytics? getAnalytics() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like Analytics
is sure to be non-null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - done.
example/example.dart
Outdated
} | ||
|
||
return _analytics; | ||
} | ||
|
||
void _handleFoo() { | ||
var analytics = getAnalytics(); | ||
var analytics = getAnalytics()!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which case this bang can go away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup - removed.
lib/src/usage_impl.dart
Outdated
int drops; | ||
int _lastReplenish; | ||
late int drops; | ||
late int _lastReplenish; | ||
|
||
ThrottlingBucket(this.startingCount) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider assigning drops
in an initializer list. Then you can drop the late
modifier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 - yup, this works
lib/src/usage_impl_io.dart
Outdated
|
||
@override | ||
void operator []=(String key, dynamic value) { | ||
if (value == null && !_map.containsKey(key)) return; | ||
if (_map[key] == value) return; | ||
if (value == null && !_map!.containsKey(key)) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be inclined to mark _map
late so you can drop all of these bangs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pubspec.yaml
Outdated
|
||
dependencies: | ||
path: ^1.4.0 | ||
path: ^1.8.0-nullsafety.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious: why the .3
lower bound?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just the latest available - I can drop to 1.8.0-nullsafety
.
Thanks for the review! Updated with the latest - ptal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍