Skip to content

Fixed LogLevel ordering and added tests#2102

Merged
NachoSoto merged 9 commits into
mainfrom
log-level-ordering
Nov 30, 2022
Merged

Fixed LogLevel ordering and added tests#2102
NachoSoto merged 9 commits into
mainfrom
log-level-ordering

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Nov 29, 2022

Copy link
Copy Markdown
Contributor

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
Fixed LogLevel ordering and added tests
41ce4b6 
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 NachoSoto requested a review from a team November 29, 2022 21:43
@NachoSoto NachoSoto added the test label Nov 29, 2022
@NachoSoto

NachoSoto commented Nov 29, 2022

Copy link
Copy Markdown
Contributor Author

Didn't add a fix label to this because technically we never shipped this wrong on any release.

NachoSoto
NachoSoto commented Nov 29, 2022
View reviewed changes
Comment thread Tests/UnitTests/TestHelpers/TestLogHandler.swift Outdated
@NachoSoto NachoSoto merged commit 872b4cf into main Nov 30, 2022
@NachoSoto NachoSoto deleted the log-level-ordering branch November 30, 2022 14:17
@RCGitBot RCGitBot mentioned this pull request Nov 30, 2022
Merged
NachoSoto added a commit that referenced this pull request Dec 9, 2022
@NachoSoto
PostReceiptDataOperation: also print receipt data with verbose logs
b1d5f8d 
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 NachoSoto mentioned this pull request Dec 9, 2022
Merged
NachoSoto added a commit that referenced this pull request Dec 13, 2022
@NachoSoto
PostReceiptDataOperation: also print receipt data with verbose lo…
583e2a3 
…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

Reviewers

@vegaro vegaro vegaro approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@NachoSoto @vegaro