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

Jump to bottom

Make package scriptlets more universal #580

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make package scriptlets more universal #580

wants to merge 1 commit into from

Conversation

iavael
Copy link

@iavael iavael commented Jun 19, 2023

Package scriplets may fail on some system for various reasons like:

  • restricted systemctl commands on systems like fedora silverblue
  • lack of systemd overall on systems like devuan

So I made command invocation allowed to fail to make package scriptlets more robust in such edge cases.
And also removed unnecessary subshell invocation.

TheAssassin
TheAssassin requested changes Jul 10, 2023
View reviewed changes
Copy link
Owner

@TheAssassin TheAssassin left a comment

Choose a reason for hiding this comment

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

Thanks. You have a few good points. Please see my review.

resources/install-scripts/post-install.in Outdated Show resolved Hide resolved
resources/install-scripts/post-install.in Outdated Show resolved Hide resolved
resources/install-scripts/post-install.in Outdated Show resolved Hide resolved
@iavael
Copy link
Author

iavael commented Jul 10, 2023

@TheAssassin I'll take a deeper look during this week because I have some issues with broadband internet connection now.

@iavael iavael requested a review from TheAssassin September 28, 2023 22:47
@TheAssassin
Copy link
Owner

Thanks, I already left some comments.

@iavael iavael force-pushed the fix-scriptlets-1 branch 3 times, most recently from 9d45e1c to c37fa87 Compare September 28, 2023 23:35
@iavael
Copy link
Author

iavael commented Sep 28, 2023

Ok, I've finished all adjustments. Ready for review.

@iavael
Copy link
Author

iavael commented Oct 5, 2023

@TheAssassin in case, if message about adjustments had been finished got lost among notifications, humbly remind you about my PR being ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheAssassin TheAssassin Awaiting requested review from TheAssassin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iavael @TheAssassin