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.

Sign up for GitHub

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

Jump to bottom

Log errored key ids during import/merge. #1166

Merged
merged 1 commit into from
Jun 6, 2020
Merged

Log errored key ids during import/merge. #1166

merged 1 commit into from
Jun 6, 2020

Conversation

ni4
Copy link
Contributor

@ni4 ni4 commented Jun 5, 2020

This PR improves logging, making potential error discovery faster and easier.
Fixes #1162

@ni4 ni4 mentioned this pull request Jun 5, 2020
Closed
@ronaldtse
Copy link
Contributor

@ni4 would it be better to do less preprocessor magic? Wouldn't the compiler optimize those anyway?

@rrrooommmaaa
Copy link
Contributor

@ni4 would it be better to do less preprocessor magic? Wouldn't the compiler optimize those anyway?

inline function, probably?

rrrooommmaaa
rrrooommmaaa approved these changes Jun 6, 2020
View reviewed changes
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa left a comment

Choose a reason for hiding this comment

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

LGTM!

ronaldtse
ronaldtse approved these changes Jun 6, 2020
View reviewed changes
Copy link
Contributor

@ronaldtse ronaldtse left a comment

Choose a reason for hiding this comment

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

Let's merge first anyway.

@ronaldtse ronaldtse merged commit 274f601 into master Jun 6, 2020
@ronaldtse ronaldtse deleted the ni4-1162-print-errored-keyid branch June 6, 2020 17:02
@ni4
Copy link
Contributor Author

ni4 commented Jun 8, 2020

@ni4 would it be better to do less preprocessor magic? Wouldn't the compiler optimize those anyway?

I don't like defines approach equally to code duplication, but in this case it gives the ability to correctly print source file/function/line number. When we'll migrate to C++ so all exceptions are caught this could be replaced with const std::string &key.keyidhex().

@ronaldtse
Copy link
Contributor

@ni4 sounds good 👍

@ni4 ni4 mentioned this pull request Jun 8, 2020
Merged
@ni4 ni4 added this to the v0.14.0 milestone Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrrooommmaaa rrrooommmaaa rrrooommmaaa approved these changes

@ronaldtse ronaldtse ronaldtse approved these changes

@dewyatt dewyatt Awaiting requested review from dewyatt

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.14.0
Development

Successfully merging this pull request may close these issues.

Log offending key ID on import failure
3 participants
@ni4 @ronaldtse @rrrooommmaaa