Remove redundant install_* methods#4792
Conversation
AthreyVinay
added
step | prepare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates package installation methods into a single 'install' call, removes redundant container rebuilds in the 'bootc' manager, and optimizes 'rpm-ostree' presence checks. Feedback requires passing 'options' to 'super().install()' in 'rpm-ostree' to ensure settings are respected and including the installable name in 'apk' error messages for better clarity.
AthreyVinay
force-pushed
the
avinay-simplify-install-methods
branch
from
7adab73 to
0f5df8c
Compare
AthreyVinay
force-pushed
the
avinay-simplify-install-methods
branch
from
0f5df8c to
4812107
Compare
|
|
||
| if missing_installables: | ||
| self.engine.install(*missing_installables, options=options) | ||
| return self.build_container() or CommandOutput(stdout=None, stderr=None) |
There was a problem hiding this comment.
self.build_container() call being dropped is unexpected, what is the reason for it?
There was a problem hiding this comment.
Reason (if I remember...): PrepareInstall called install_from_repository() and install_from_url(), which didn't call build_container(). The build happened through finalize_installation() at the end. Are you hinting at other callers?
3 participants
originated from a comment here: #4605 (comment). Good to align before artifacts call various install methods.
also, a slight functional change in
RpmOstree.sort_packages()which now batches the presence check.