search

dhruv8sh / arch-update-checker

Plasma 6 applet to check for AUR and Arch updates
GNU General Public License v2.0
70 stars 5 forks source link

Replace hardcoded yay with user-config aur wrapper for single install #21

Closed Schlaefer closed 3 months ago

Schlaefer commented 3 months ago

I only took a quick glance at the code, but it seems this was missed when switching from yay to a generic wrapper?

dhruv8sh commented 3 months ago

This calls for a cleanup to the PackageManager.qml file. Thanks for pointing it out, great job!

dhruv8sh commented 3 months ago

It was cleaned up. If you have the time, please review it.

Schlaefer commented 3 months ago

It was cleaned up. If you have the time, please review it.

OK, I'm far from a QML guru, so the framing is "I know some of these words" and let's try if I can understand it. Just skimming over the code of PackageManager.qml, but since you asked here are my 5 cents. :)

Much better than before, a lot more concise.

The -e hold is still floating around to loosely. Sometimes it's term+hold, sometimes it's term + " --hold -e". This makes the code hard to evaluate. There should be one central instance that manages the --hold and independent from the -e. Consolidate all of it (term -e and hold) in the executable.exec() call. Since you're always doing it, no need to repeat and pass it in all the different function calls to exec()?

dhruv8sh commented 3 months ago

Agreed! Will make the changes.