Skip to content

Added LogLevel.verbose#2080

Merged
NachoSoto merged 4 commits into
mainfrom
verbose-log-level
Nov 26, 2022
Merged

Added LogLevel.verbose#2080
NachoSoto merged 4 commits into
mainfrom
verbose-log-level

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Nov 23, 2022

Copy link
Copy Markdown
Contributor

I want to add some extra logs to figure out CSDK-517 which would be too verbose (no pun intended) for debug, so this will allow us to have some additional logging that wouldn't be enabled by default.

@vegaro vegaro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is breaking our public API correct?

@NachoSoto

Copy link
Copy Markdown
Contributor Author

By breaking do you mean because we added a new case?
Technically yes, so we could tag this as feat if we want to be 100% correct, but I'd be surprised anyone is switching over our LogLevel?

Well I suppose users implementing their own LogHandler might.

Base automatically changed from timing-util to main November 23, 2022 17:51
@aboedo

aboedo commented Nov 24, 2022

Copy link
Copy Markdown
Member

@NachoSoto this does technically fall under feat since it's a new enum case.
But we do modify the values, so that's the breaking part, since it's an int enum and we're effectively moving them all by 1. Maybe we can just keep the existing numbering by setting it explicitly, that way we're being extra safe and not modifying an existing public value

@NachoSoto

Copy link
Copy Markdown
Contributor Author

Oh yeah that's a great point!!

I want to add some extra logs to figure out [CSDK-517] which would be too verbose (no pun intended) for `debug`, so this will allow us to have some additional logging that wouldn't be enabled by default.
@NachoSoto NachoSoto added pr:feat A new feature and removed test labels Nov 26, 2022
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Changed to stable IDs and made this feat to be 100% safe.

@NachoSoto NachoSoto enabled auto-merge (squash) November 26, 2022 00:47
@NachoSoto NachoSoto merged commit f610524 into main Nov 26, 2022
@NachoSoto NachoSoto deleted the verbose-log-level branch November 26, 2022 00:56
NachoSoto added a commit that referenced this pull request Nov 26, 2022
Depends on #2080.
This will help debug [CSDK-517].

### Examples:
```
- VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x00006000037af800)
StoreKit Wrapper: left(<RevenueCat.StoreKit1Wrapper: 0x600000b6d3a0>)
- VERBOSE: Purchases.deinit: Purchases (0x0000600003f82600)
```
NachoSoto added a commit that referenced this pull request Nov 26, 2022
…2082)

Depends on #2080.
This will help debug [CSDK-517].

### Examples:
```
- VERBOSE: Purchases.init: created new Purchases instance: Purchases (0x00006000037af800)
StoreKit Wrapper: left(<RevenueCat.StoreKit1Wrapper: 0x600000b6d3a0>)
- VERBOSE: Purchases.deinit: Purchases (0x0000600003f82600)
```

[CSDK-517]:
https://revenuecats.atlassian.net/browse/CSDK-517?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@vegaro

vegaro commented Nov 28, 2022

Copy link
Copy Markdown
Member

Thanks for explaining @aboedo , that's exactly what I meant. I should've been more clear about my concern :)

NachoSoto added a commit that referenced this pull request Nov 29, 2022
A last minute change in #2080 made `LogLevel.verbose` have a higher `rawValue` to maintain backwards compatibility.
Unfortunately, we we re relying on these to implement ordering.

This PR makes that explicit by making `LogLevel` conform to `Comparable`, and added tests to verify this behavior in `Logger`.
NachoSoto added a commit that referenced this pull request Dec 9, 2022
This was fine when we only had `debug` logs, but since #2080 we need to check for `verbose` too.
Thanks to #2102 we can just use `<=`, because `LogLevel` conforms to `Comparable`.
NachoSoto added a commit that referenced this pull request Dec 13, 2022
…gs (#2127)

This was fine when we only had `debug` logs, but since #2080 we need to
check for `verbose` too.
Thanks to #2102 we can just use `<=`, because `LogLevel` conforms to
`Comparable`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants