Add missing measurements units#1033
Conversation
| /// This is a singe enum because Enhanced Enums in Dart is only available | ||
| /// in newer versions. |
There was a problem hiding this comment.
Limitation before Dart 2.17 and we cannot do breaking changes just yet.
It's either this or an open String, but this has better UX, so users don't need to find out the units by themselves via docs.
Codecov ReportBase: 90.26% // Head: 90.48% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1033 +/- ##
==========================================
+ Coverage 90.26% 90.48% +0.21%
==========================================
Files 114 114
Lines 3556 3572 +16
==========================================
+ Hits 3210 3232 +22
+ Misses 346 340 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
philipphofmann
left a comment
There was a problem hiding this comment.
Maybe there is a different way of fixing it.
philipphofmann
left a comment
There was a problem hiding this comment.
LGTM, as discussed on a call with @marandaneto, that Dart doesn't offer inheritance for enums or method overloads, and that's why we put all types into one enum.
📜 Description
Not as nice as
SentryMeasurementUnit.duration.milliSecondas some other languages, but it's a limitation of theenumtype in Dart in the current min. supported version, we can refactor that in the future.Changing it to a plain
Stringwould be a breaking change and would have a poor UX because we'd need to document all the possible values.The current limitation is that you can only set the unit with one of the enum values, which is well covered for most of the cases.
In case somebody wants to use their own metric unit, it's currently not supported, we can refactor this and bump the Dart version if needed.
💡 Motivation and Context
Reason: getsentry/sentry-java#2260 (comment)
💚 How did you test it?
📝 Checklist
🔮 Next steps