Skip to content
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

Logging v2 part 2 #399

Merged
merged 4 commits into from
Nov 20, 2020
Merged

Logging v2 part 2 #399

merged 4 commits into from
Nov 20, 2020

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Nov 17, 2020

Improved API for logging in Swift.
Added a few log entries to Swift code.

Requirements:

@aboedo aboedo added the HOLD label Nov 17, 2020
@aboedo aboedo requested a review from vegaro November 17, 2020 18:23
@aboedo aboedo self-assigned this Nov 17, 2020
Comment on lines +588 to 594
RCWarnLog(@"allowSharingAppStoreAccount is set to false and restoreTransactions has been called. "
"Are you sure you want to do this?");
Copy link
Member Author

Choose a reason for hiding this comment

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

these feel more like warnings to me, but we didn't use to have a warn level

@@ -51,7 +51,7 @@ extension ProductsManager: SKProductsRequestDelegate {

func productsRequest(_ request: SKProductsRequest, didReceive response: SKProductsResponse) {
queue.async { [self] in
NSLog("products request received response")
Logger.info("products request received response")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be debug?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm honestly not sure which is best here. I was kind of torn because of stuff like #250. I might improve the message a bit so that it's more useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

I should also make it consistent with line 70, where I'm using debug, so that either they're both debug or both info

Base automatically changed from logging_v2 to develop November 20, 2020 17:49
@aboedo aboedo removed the HOLD label Nov 20, 2020
@aboedo aboedo merged commit c84372d into develop Nov 20, 2020
@aboedo aboedo deleted the logging_v2_part_2 branch November 20, 2020 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants