Change DDLogLevel from enum to options
#1249
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1249 +/- ##
==========================================
- Coverage 47.58% 45.41% -2.18%
==========================================
Files 32 26 -6
Lines 5174 4391 -783
Branches 484 0 -484
==========================================
- Hits 2462 1994 -468
+ Misses 2540 2397 -143
+ Partials 172 0 -172
Continue to review full report at Codecov.
|
|
@nhgrif Thanks for proposing and implementing this change! For the rest of the team here: I'd be somewhat in favor of it being defined as @nhgrif The easy part is done. 😉
|
|
@nhgrif I just realized it should probably be This documentation shows the differences between the various options. |
As discussed in [this StackOverflow question](https://stackoverflow.com/questions/68854372/how-to-implement-custom-log-levels-in-cocoalumberjack-from-swift), there's not really a clean way to implement custom log levels in Swift. As `DDLogFlag` is defined as `NS_OPTIONS`, absolutely any value is valid here, but with `DDLogLevel` defined as an enum, as far as Swift as concerned the existing levels are the only valid levels. I can't even particularly cleanly write aliases for the existing levels. Further, to me, it doesn't even make particular sense for `DDLogFlag` to be `NS_OPTIONS` while `DDLogLevel` is `NS_ENUM`. If anything, shouldn't it almost be the opposite...? Logically, it doesn't make sense that something might be logged as `[.error, .info]` from Swift, but you could log something using that flag... but what does that even mean? Meanwhile, from Swift, I can not do as some CocoaLumberjack documentation suggests, and easily filter to say "show me `.info` level, but not `.warnings`, but do show me `.error` level". This again comes back to the fact that `DDLogLevel` is an enum and from the Swift side I can not create any custom values for this. By this logic, `DDLogFlag` itself should arguably be `NS_ENUM` rather than `NS_OPTION`, however, I recommend leaving it as `NS_OPTION`, because changing it to `NS_ENUM` once again would limit my ability to use custom log levels from Swift.
New Pull Request Checklist
I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (
pod lib lint)This merge request fixes / refers to the following issues: n/a
Pull Request Description
As discussed in this StackOverflow question, there's not really a clean way to implement custom log levels in Swift.
As
DDLogFlagis defined asNS_OPTIONS, absolutely any value is valid here, but withDDLogLeveldefined as an enum, as far as Swift as concerned the existing levels are the only valid levels. I can't even particularly cleanly write aliases for the existing levels.Further, to me, it doesn't even make particular sense for
DDLogFlagto beNS_OPTIONSwhileDDLogLevelisNS_ENUM. If anything, shouldn't it almost be the opposite...? Logically, it doesn't make sense that something might be logged as[.error, .info]from Swift, but you could log something using that flag... but what does that even mean? Meanwhile, from Swift, I can not do as some CocoaLumberjack documentation suggests, and easily filter to say "show me.infolevel, but not.warnings, but do show me.errorlevel". This again comes back to the fact thatDDLogLevelis an enum and from the Swift side I can not create any custom values for this.By this logic,
DDLogFlagitself should arguably beNS_ENUMrather thanNS_OPTION, however, I recommend leaving it asNS_OPTION, because changing it toNS_ENUMonce again would limit my ability to use custom log levels from Swift.