Fix dynamic logic for ICE stats #239
Conversation
| @@ -234,11 +234,11 @@ Future _logConnectionStats(Stopwatch webrtcDialSW, RTCPeerConnection peerConnect | |||
| if (stat.type == 'candidate-pair' && stat.values['nominated']) { | |||
| // Use 'lastPacketSentTimestamp' on candidate pair to estimate when the | |||
| // pair was nominated. | |||
| final double lpst = stat.values['lastPacketSentTimestamp']; | |||
| final double lpst = stat.values['lastPacketSentTimestamp'] ?? 0; | |||
There was a problem hiding this comment.
[nit] not set on the ?? 0 if there is a better default value - or if we should set it to null to do some exception handling than that may be preferred. I'll leave that up to you though!
There was a problem hiding this comment.
Ah I think I'm fine with the ?? 0.
| final lcid = stat.values['localCandidateId']; | ||
| final rcid = stat.values['remoteCandidateId']; |
There was a problem hiding this comment.
are you comfortable with these being instantiated as dynamic? you'll need to cast them later, I think.
if not, you could do like above:
| final lcid = stat.values['localCandidateId']; | |
| final rcid = stat.values['remoteCandidateId']; | |
| final String? lcid = stat.values['localCandidateId']; | |
| final String? rcid = stat.values['remoteCandidateId']; |
or
| final lcid = stat.values['localCandidateId']; | |
| final rcid = stat.values['remoteCandidateId']; | |
| final lcid = stat.values['localCandidateId'] ?? ''; // or some default value | |
| final rcid = stat.values['remoteCandidateId'] ?? ''; |
There was a problem hiding this comment.
Interesting... they seem to string-interpolate just fine as dynamics; is it preferred for them to be String? or String?
There was a problem hiding this comment.
I'm not familiar with Dart, but was there a reason to remove the String type?
There was a problem hiding this comment.
They are not in fact Strings; they are of type dynamic and could be cast to String? (nullable String).
| @@ -27,7 +27,7 @@ jobs: | |||
| - name: Test HTML | |||
| # https://github.com/wjdp/htmltest-action/ | |||
| # Don't fail the build on broken links | |||
| continue-on-error: false | |||
| continue-on-error: true | |||
There was a problem hiding this comment.
We were getting CI failures due to a broken link; given the comment above, I think this should be true.
No description provided.