mcgillij / amdfan

Updated AMD Fan control utility forked from amdgpu-fan and updated.
https://mcgillij.dev/pages/amdfan.html
GNU General Public License v2.0
33 stars 7 forks source link

improve service support, + miscellaneous changes #32

Closed mazunki closed 3 months ago

mazunki commented 4 months ago
mazunki commented 4 months ago

whoops, i didn't mean to request a merge for this yet, since it's a bit bundled together and out of order. was too quick on running github-cli while preparing it

i initially started working on adding support for pidfile because that makes service managers happy, and one thing led to another lol.

while i haven't changed any implementation behaviour, this PR is a breaking change. commands are no longer flags, for instance. i believe i have documented all the new things, though. if i missed something, please let me know.

i realize this PR would be best split into two different PRs, sorry about that. let me know if something should be changed :)

changes:

mazunki commented 4 months ago

i think it would be wise to update and aggregate to the tests too. i only updated the tests which already existed

mcgillij commented 3 months ago

Cool I'll take a look, sorry was out on vacation this week, so haven't kept up with GH.

mcgillij commented 3 months ago

Looks good, I'll have to pull it down to make sure I can still build it for the Arch packaging, and to make sure it still works with my py3status-amdfan, I'll likely just have to modify some of the commands on the widget side of things.

mazunki commented 3 months ago

Oh, lovely, thanks for merging it. :)

Sorry about the delay myself, I have been AFK too. Is there anything left to do/change? I know the tests should probably be updated... but that's probably for another time.

It seems you got the PKGBUILD covered? If you think it's ready, I will bump the ebuild for Gentoo on ::guru too.

mcgillij commented 3 months ago

yeah I fixed at least the existing tests, so that the right cli funcs are passed so we no longer have the 2 == 1 etc.

The rest of the changes were just to make it work with the github workflow build.