-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Minor refactoring of Formatter logic #2998
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2998 +/- ##
==========================================
+ Coverage 19.45% 20.77% +1.31%
==========================================
Files 113 113
Lines 16044 16448 +404
Branches 247 289 +42
==========================================
+ Hits 3122 3417 +295
- Misses 12884 12992 +108
- Partials 38 39 +1
Continue to review full report at Codecov.
|
{ | ||
return formatter?.string(from: NSNumber(floatLiteral: value)) ?? "" | ||
} | ||
return block?(value, axis) ?? |
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 think if let
would be more readable in this case.
{ | ||
return formatter?.string(from: NSNumber(floatLiteral: value)) ?? "" | ||
} | ||
return block?(value, entry, dataSetIndex, viewPortHandler) ?? |
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 think if let
would be more readable in this case.
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 flip-flopped on this one a lot. I'll change it now.
Changes have been made |
@petester42 Any further feedback on this one or can it be merged? |
Actually the naming of the commit / PR itself could have been better, as it does not change the logic - it's a refactoring :-) |
and I already told him to try squash before merging and our idea for managing commits |
No description provided.