Updated log level presets to better suit default needs (#91) #93
Conversation
ssawchenko
changed the title
| case .debugVerbose: return .none | ||
| case .debug: return .none | ||
| case .release: return .info | ||
| case .releaseAdvanced: return .warn |
There was a problem hiding this comment.
I think this should be info - isn't the intent that releaseAdvanced returns more info than release?
There was a problem hiding this comment.
Good catch. Looks like based on the table I pulled from our meeting this should be debug. I'll go through again with a fresh set of eyes and make sure that all lines up properly.
| } | ||
| } | ||
|
|
||
| // TODO 2021-09-16: If analytics are by default no longer supported, should we remove this? |
There was a problem hiding this comment.
Yeah, I think we should just chop this out
There was a problem hiding this comment.
I have a ticket to remove track (#96), so I'll pull this code out then as well.
| /// Disk: verbose, system: none, remote: warn, analytics: enabled | ||
| case releaseAdvanced | ||
| /// Disk: none, system: none, remote: warn, analytics: enabled | ||
|
|
There was a problem hiding this comment.
was there a good argument for removing the comments here? I think theres a good case for keeping them around, since they show up in the doc comments when you select one of the presets in Xcode.
There was a problem hiding this comment.
It just seemed like duplication of the settings below, and a place where we may end up with discrepancies if it's changed in one place and not the other. It didn't occur to me that this was removing the linked comments in the predictive text (? I can't recall the actual term) functionality. I'll add them back.
There was a problem hiding this comment.
Added comments back!
|
@brendanlensink Please see addressed comments, ready for another review :) |
Related Issue: #91
Summary of Problem:
In our TechWG meeting where we reviewed Steamclog usage, we decided on some updates to how LogLevelPresets were named and what their LogLevel mappings were:
Tech WG Doc:
https://coda.io/d/Tech-Working-Group_dq8QHMnEi6A/Aug-16th-2021_suivT#_luAHl
Proposed Solution: