PackagesProposal - Added API for optional resolvables #519
Conversation
(fixup for bsc#885496)
| @@ -32,8 +32,11 @@ library/*/testsuite/*.sum | |||
| library/*/testsuite/*.exp | |||
| library/*/testsuite/*.bak | |||
| library/*/test/*.log | |||
| library/*/test/*/*.log | |||
There was a problem hiding this comment.
what about modifying above to library/*/test/**/*.log
There was a problem hiding this comment.
Yes, that's better, I didn't know that ** is supported...
| library/*/test/*.trs | ||
| library/*/test/*/*.trs |
| if @resolvables_to_install != {} | ||
| Builtins.y2warning("Reseting all PackagesProposal items") | ||
| if !@resolvables_to_install.empty? || !@opt_resolvables_to_install.empty? | ||
| log.warn("Resetting all PackagesProposal items") |
There was a problem hiding this comment.
why it is warning? you call method to reset it and it warn if it reset something?
| @@ -73,34 +78,29 @@ def GetSupportedResolvables | |||
|
|
|||
| def IsSupportedResolvableType(type) | |||
| if type.nil? | |||
| Builtins.y2error("Wrong type: %1", type) | |||
| log.error("Type cannot be nil") | |||
There was a problem hiding this comment.
I think below return is not needed and can be only one liner.
| ) | ||
| log.info("Adding #{log_label(optional)} #{resolvables} of type #{type} for #{unique_ID}") | ||
|
|
||
| data(optional)[unique_ID][type].concat(resolvables) |
There was a problem hiding this comment.
it would be nice if CreateEmptyStructureIfMissing(unique_ID, type, optional: optional) just return that data(optional)[unique_ID][type] so maybe better name would be data_for_uid_and_type which create it if needed.
| # sort the result and remove the duplicates | ||
| ret.sort! | ||
| ret.uniq! | ||
|
|
||
| deep_copy(ret) |
There was a problem hiding this comment.
is deep copy needed? do you expect someone will modify strings?
| end | ||
|
|
||
| deep_copy(ret) | ||
| ret | ||
| end | ||
|
|
||
| # Return whether a unique ID is already in use. |
There was a problem hiding this comment.
in fact this is quite confusing, even if cleared in return part of doc
lslezak
force-pushed
the
optional_patterns
branch
from
37ab87e
to
e12edb5
Compare
|
in proposal more removed lines then added, so LGTM for me. Just coverage decrease looks a bit suspicious. |
This API enhancement is needed by
yast2-packagerto fix blocked installation after deselecting some default patterns in openQA tests (YaST complains about missing patterns and blocks the installation).BuiltinsandConvertcallsPackagesProposalmodule (covers 100% of the module)