-
Notifications
You must be signed in to change notification settings - Fork 139
Make logging thread-safe and the log level runtime-configurable #2834
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
Changes from all commits
6141f4b
f9fff3b
98b59c2
aa61765
3172762
81168b4
2f4a6a6
a48924d
f9f1fb1
0f9ab99
6c48813
f94abcd
44ee530
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -588,7 +588,7 @@ TEST(IndexTest, trivialGettersAndSetters) { | |
| } | ||
|
|
||
| TEST(IndexTest, updateInputFileSpecificationsAndLog) { | ||
| SKIP_IF_LOGLEVEL_IS_LOWER(WARN); | ||
| SKIP_IF_LOGLEVEL_IS_LOWER(INFO); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change intentional?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is intentional, as it was wrong and this test in fact fails when being run on loglevel WARN, which was detected by the new infrastructure (which sets the loglevel as part of the macro).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that part of the test requires info while another part only requires warn (the deprecation messages). |
||
| using enum qlever::Filetype; | ||
| std::vector<qlever::InputFileSpecification> singleFileSpec = { | ||
| {"singleFile.ttl", Turtle, std::nullopt}}; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no cleaner way to do this? For example, do we really need all the methods in
Fileto be in the.hfile? And I only see oneAD_LOG_...in the file, inside of thedeleteFilemethod, which is certainly not performance-criticalThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved the
#defineto theCMakeLists.txt. Then we can safely also directly include thenlohmann/json.hpp. Theutil/jsohcontains some advanced heljper functions which we don't need here anyway, so that solution is fine for now, and the ugly part is gone.