(PA-5786) Add ability to execute direct post installation scriptlets#820
Conversation
tvpartytonight
force-pushed
the
PA-5786_alt
branch
from
c763b1f to
8365247
Compare
|
This might be preferable to #819 as it should be safer and would only affect projects using |
tvpartytonight
force-pushed
the
PA-5786_alt
branch
from
8365247 to
d5e6980
Compare
e-gris
left a comment
e-gris
left a comment
There was a problem hiding this comment.
A couple of ignorable nitpicky style comments but otherwise LGTM.
I actually liked the previous one better because if the cleanliness it introduced, but understand the reasoning to head this way to avoid breaking things.
lib/vanagon/component/dsl.rb
Outdated
| pkg_state = Array(pkg_state) | ||
| scripts = Array(scripts) |
There was a problem hiding this comment.
Style only: don't mutate caller-supplied arguments; create new variables instead.
lib/vanagon/project.rb
Outdated
| scripts = components.flat_map(&:postinstall_required_actions).compact.select { |s| s.pkg_state.include? pkg_state }.map(&:scripts) | ||
| if scripts.empty? | ||
| return ': no postinstall required scripts provided' | ||
| else |
There was a problem hiding this comment.
Style only: else is redundant. Favor a guardian clause:
return ': no postinstall required scripts provided' if scripts.empty?
[return] scripts.join("\n")The 2nd return is optional depending on your aesthetic preferences.
lib/vanagon/project.rb
Outdated
| # @return [String] string of Bourne shell compatible scriptlets to execute during the postinstall | ||
| # phase of packaging during the state of the system defined by pkg_state (either install or upgrade) | ||
| def get_postinstall_actions(pkg_state) | ||
| def get_postinstall_required_actions(pkg_state) |
There was a problem hiding this comment.
This is titled incorrectly and should be the name of the function below.
tvpartytonight
force-pushed
the
PA-5786_alt
branch
2 times, most recently
from
898dfaf to
1f169dc
Compare
|
Also, when ready, please add a CHANGELOG update. |
tvpartytonight
force-pushed
the
PA-5786_alt
branch
from
1f169dc to
a4045d5
Compare
|
@e-gris I think I have address all comments, let me know if there is anything I missed. |
|
Put an '### Added' header above the changelog entry and all will be well. |
The existing `#add_postinstall_action` adds actions to the `%posttrans` section of an rpm spec file. This can mean that the action could fail, but the installation/upgrade could still succeed. This commit adds the ability to add actions that must succeed post installation for rpm installations by adding the action to the `%post` section.
tvpartytonight
force-pushed
the
PA-5786_alt
branch
from
a4045d5 to
93ba1b4
Compare
3 participants
The existing
#add_postinstall_actionadds actions to the%posttranssection of an rpm spec file. This can mean that the action could fail, but the installation/upgrade could still succeed. This commit adds the ability to add actions that must succeed post installation for rpm installations by adding the action to the%postsection.Please add all notable changes to the "Unreleased" section of the CHANGELOG.