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

Fixes an outrageous number of bugs and warnings #90

Merged
merged 23 commits into from
Jan 13, 2025
+524 −325
Merged

Fixes an outrageous number of bugs and warnings #90

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d964188
Fixes an outragous number of bugs and warnings
viniciusferrao Jan 8, 2025
233c439
Fixed more outrageous bugs.
viniciusferrao Jan 9, 2025
d7dcb49
Disable cosmetic warnings
Jan 9, 2025
61a01f3
Pin spdlog version
Jan 9, 2025
527bd42
Fix warning
Jan 9, 2025
2de5a13
Run clang-format
Jan 9, 2025
b2c1bb6
Handle copy to temporary file error
Jan 9, 2025
75c03af
Fix message in CMakeLists.txt
Jan 9, 2025
a892e84
Handle error in answerfile minor version
Jan 9, 2025
cc06a6c
Fix routing error disabling the VM network
Jan 9, 2025
1f55fdf
Enable Managed network again
Jan 9, 2025
8712198
Fix answerfile version handling
Jan 9, 2025
fb6231a
Fix copyFile implementation
Jan 10, 2025
3dfb889
Fix typos, change variables names and fix throw.
viniciusferrao Jan 10, 2025
0bfe8ee
Clearning more warnings
Jan 10, 2025
121594c
Fix the last warnings
Jan 11, 2025
5c3012a
Remove std::exit call, and raise an exception instead
Jan 11, 2025
66ce55b
Run clang-format
Jan 13, 2025
a659471
Added version ranges to conanfile.txt
viniciusferrao Jan 13, 2025
69d6dac
Tries to update the clang-format.yml workflow
viniciusferrao Jan 13, 2025
39ba2c1
Try to fix clang-format again
viniciusferrao Jan 13, 2025
bb0eae7
Last try to fix clang-format
viniciusferrao Jan 13, 2025
b6a6fa0
Final try to fix clang-format. I promise.
viniciusferrao Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions include/cloysterhpc/services/log.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@
*
* @param __VA_ARGS__ The message to log and its format arguments.
*/
#define LOG_ABORT(...) \
if (spdlog::get(productName) != nullptr) { \
spdlog::get(productName)->critical(__VA_ARGS__); \
std::exit(-1); \
}
#define LOG_CRITICAL(...) \
if (spdlog::get(productName) != nullptr) { \
spdlog::get(productName)->critical(__VA_ARGS__); \
Expand All @@ -45,7 +40,12 @@
if (spdlog::get(productName) != nullptr) { \
spdlog::get(productName)->info(__VA_ARGS__); \
}

#define LOG_ABORT_IF(cnd, msg) \
Copy link
Owner Author

@viniciusferrao viniciusferrao Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't like the idea of adding logic to a macro.

The idea here is to just print the logs. That's the single responsibility. If you need to do a check, do it on code, print a log an then exits.

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-macros2

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay I'll remove the macro.

if ((cnd) && spdlog::get(productName) != nullptr) { \
LOG_CRITICAL("ABORT - {}\n\t{}\n\tin file: {}\n\ton line: {}", #cnd, \
msg, __FILE__, __LINE__); \
LOG_BREAK; \
}
// Available only with DEBUG builds
#ifndef NDEBUG
#define LOG_DEBUG(...) \
Expand Down
7 changes: 3 additions & 4 deletions src/os.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,9 @@ void OS::setVersion(const std::string& version)
setMajorVersion(static_cast<unsigned>(
std::stoul(version.substr(0, version.find('.')))));


if (version.find('.') != std::string::npos) {
LOG_ABORT("system version (in the answerfile.yml) must follow the format M.N, where M is the major version number and N is the minor version number.");
}
LOG_ABORT_IF(version.find('.') == std::string::npos,
Copy link
Owner Author

Choose a reason for hiding this comment

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

That's what I'm talking about. Use a if statement and does not hide it behind a macro.

"system version (in the answerfile.yml) must follow the format M.N, "
"where M is the major version number and N is the minor version number.");

setMinorVersion(
static_cast<unsigned>(stoul(version.substr(version.find('.') + 1))));
Expand Down
Loading