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

Add LogMessage class to vespalog. #8895

Merged
merged 4 commits into from Mar 26, 2019

Conversation

Projects
None yet
2 participants
@toregge
Copy link
Member

toregge commented Mar 25, 2019

@geirst : please review

@toregge toregge requested a review from geirst Mar 25, 2019

@geirst
Copy link
Member

geirst left a comment

Overall looks good, with some comments to consider and address.

Show resolved Hide resolved vespalog/src/test/log_message/log_message_test.cpp Outdated
Show resolved Hide resolved vespalog/src/test/log_message/log_message_test.cpp Outdated
Show resolved Hide resolved vespalog/src/test/log_message/log_message_test.cpp Outdated
Show resolved Hide resolved vespalog/src/vespa/log/exceptions.h
Show resolved Hide resolved vespalog/src/test/log_message/log_message_test.cpp
Show resolved Hide resolved vespalog/src/vespa/log/log_message.cpp Outdated
Show resolved Hide resolved vespalog/src/vespa/log/log_message.cpp Outdated
Show resolved Hide resolved vespalog/src/vespa/log/log_message.cpp Outdated
Show resolved Hide resolved vespalog/src/test/log_message/log_message_test.cpp Outdated
public:
LogMessage();
~LogMessage();
void parse_log_line(std::string_view log_line);

This comment has been minimized.

Copy link
@geirst

geirst Mar 26, 2019

Member

We should consider making this static.

toregge added some commits Mar 26, 2019

Stop checking for too old or too new timestamp.
Don't allow pid field to end with trailing slash when no thread id is present.
@geirst

geirst approved these changes Mar 26, 2019

Copy link
Member

geirst left a comment

👍

@geirst geirst merged commit 6b1974f into master Mar 26, 2019

3 checks passed

Merge Stop Enforcer Check preventing merges at merge stop.
cla/licenses User has a valid Oath CLA
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@geirst geirst deleted the toregge/add-log-message-class-to-vespalog branch Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.