-
Notifications
You must be signed in to change notification settings - Fork 94
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
Fix and use ModuleProfile::is_default()
method
#988
Fix and use ModuleProfile::is_default()
method
#988
Conversation
db75bfa
to
da751bf
Compare
const auto & modulemd_profile = static_cast<ModulemdProfile *>(g_ptr_array_index(profiles, i)); | ||
result_profiles.push_back(ModuleProfile( | ||
modulemd_profile, | ||
std::find(default_profiles.begin(), default_profiles.end(), modulemd_profile_get_name(modulemd_profile)) != |
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.
This is not wrong, but written code in that way might be less readable (this is a personal opinion).
std::find(default_profiles.begin(), default_profiles.end(), modulemd_profile_get_name(modulemd_profile)) !=
default_profiles.end()));
It is possible to use temporal variable
bool is_default = std::find(default_profiles.begin(), default_profiles.end(), modulemd_profile_get_name(modulemd_profile)) !=
default_profiles.end()));
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.
Indeed, temporals are a simple yet powerful way to make complex conditions far more readable, even obvious.
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.
Sure, fixed.
The previously used `modulemd_profile_is_default` gets the information about defaults from modulemd-packager yaml document, which is available when building a module, but doesn't work for modules after building.
da751bf
to
e1519dc
Compare
LGTM |
@pkratoch May I ask you whether there is any blocker why CI tests for module list cannot be enabled |
Oh, good point. I'll take a look at which tests could be enabled now. |
35c6491
No description provided.