Electric-Coin-Company / zcash-swift-wallet-sdk

iOS light client Framework proof-of-concept
MIT License
37 stars 33 forks source link

[#1013] - Enable more granular control over logging behavior #1014

Closed tw0po1nt closed 1 year ago

tw0po1nt commented 1 year ago

The Initializer struct now takes a LoggingPolicy parameter, which enables the following options when initializing the SDK:

Closes #1013

This code review checklist is intended to serve as a starting point for the author and reviewer, although it may not be appropriate for all types of changes (e.g. fixing a spelling typo in documentation). For more in-depth discussion of how we think about code review, please see Code Review Guidelines.

Author

Reviewer

Chlup commented 1 year ago

I have two small comments otherwise this looks ok.

tw0po1nt commented 1 year ago

@Chlup @LukasKorba Since this is my first PR in this repo, I just have a couple questions.

Chlup commented 1 year ago

@Chlup @LukasKorba Since this is my first PR in this repo, I just have a couple questions.

  • Do you guys usually have the PR opener merge, or do you have another maintainer manage that?
  • In the contributing docs, it mentions to keep the change log up to date, but it doesn't look like some of the other open PRs have updated that. Would you like me to update that prior to merging this?

Hi Matthew,

PR opener usually merges the PR after it's approved by some maintainer. Changelog is usually updated for the PRs that change public API. Which this does. Please update CHANGELOG.md file accordingly.

Before merging the PR please make sure that all the targets can be compiled. You can use All scheme for that. And please be sure that tests work. Unfortunately CI currently doesn't run all the tests. Running the DarksideTests can be little challenging. But we can help with that.

Have a nice day. Michal

Chlup commented 1 year ago

I checked the tests and all work fine. Just please fix the compilation of the sample app and you should be good to go.