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.

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

Convert bash to POSIX sh, polish things #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rgcv
Copy link

@rgcv rgcv commented Nov 9, 2022

Hey there,

First off, thank you so very much for your efforts on putting this together.

I'd just like to add a small contribution by transforming some things in the
script, mostly to improve legibility and portability, to a degree. With the
latter kind of improvement, hopefully, by default, one gets better performance
out of it.

By opting for writing using POSIX shell constructs only, we strip away the
adages bash brings into the fold while still keeping the same functionality,
making it more portable, and, in systems where /bin/sh links to a shell other
than bash (say dash), we typically get better performance right off the bat
due to the lighter nature of shells such as dash.

Hopefully this PR can introduce some discussion as well, the changes are...
quite daunting at first, but from the little testing I've done, things work just
fine!

Cheers.

@steadfasterX
Copy link
Owner

many thanks for your contribution here. still thought nobody else than me uses uAu ;)

anyways: I will go through your changes asap but just wanted to let you know that this might take some time until I find some spare time for that... :(

@rgcv
Copy link
Author

rgcv commented Nov 16, 2022

many thanks

My pleasure!

thought nobody else than me uses uAu

Well, to be honest, I was looking for an unattended updater solution for arch since I'm required to do have a mechanism in place. I'm glad and grateful to you for stumbling onto this project! I typically manage updates manually without a hassle since I don't really have any unattended arch instances, they're my desktop/laptop boxes, so I don't necessarily need, e.g., mailing (though cool integration!).

just wanted to let you know that this might take some time until I find some spare time

Totally fine! I have a local version of it running, but am unable to test the emailing portion. Since no particular logic was changed in that regard (or so I hope I didn't break anything 🤞), it should be good. But yes, whenever you find time to review changes, please do! No rush ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants