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

Improved API parsing logs #779

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Conversation

michalrentka
Copy link
Contributor

No description provided.

Copy link
Contributor

@mvasilak mvasilak left a comment

Choose a reason for hiding this comment

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

Rather than having all error log messages verbosely use the same format
<response name> missing key \"<key name>\", maybe the extra argument could be responseName: String instead of errorLogMessage: String, and build the error message with that?

Even better, just add an extra convenience method

    static func parse<T>(key: String, from data: [String: Any], responseName: String) throws -> T {
        try Self.parse(key: key, from: data, errorLogMessage: "\(responseName) missing key \"\(key)\"")
    }

So we could easily differentiate the message if need be in the future?

@michalrentka
Copy link
Contributor Author

Rather than having all error log messages verbosely use the same format <response name> missing key \"<key name>\", maybe the extra argument could be responseName: String instead of errorLogMessage: String, and build the error message with that?

Even better, just add an extra convenience method

    static func parse<T>(key: String, from data: [String: Any], responseName: String) throws -> T {
        try Self.parse(key: key, from: data, errorLogMessage: "\(responseName) missing key \"\(key)\"")
    }

So we could easily differentiate the message if need be in the future?

I didn't really know what message I'd use initially so I just added a String and ended up using the same message everywhere :). Anyway I changed it to pass Class.Type so that you get autocomplete and avoid typos. If we want different message somewhere sometime in future we can overload the function. See 929316d

@michalrentka michalrentka merged commit fab9891 into zotero:master Oct 11, 2023
1 check failed
@michalrentka michalrentka deleted the improvedlogs branch October 11, 2023 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants