search

nan0s7 / nfancurve

A small and lightweight POSIX script for using a custom fan curve in Linux for those with an Nvidia GPU.
GNU General Public License v3.0
314 stars 57 forks source link

Add a picture to make it more awesome than what it already is #14

Closed wonderingabout closed 5 years ago

wonderingabout commented 5 years ago

Hi !!

After some long digging i finally found such an amazing script !!!

i customized it for my use, but i'm suggesting you take only the screenshot from my branch to illustrate better :100:

general look here : https://github.com/wonderingabout/nfancurve/tree/patch-2

the rest of my branch is not needed i think (except maybe some README clearer text, but i may submit a clearer PR for that)

also, naming temp and update is not intuitive, consider renaming it simply foreground.sh and background.sh

really thanks again, your script is very helpful and amazing, keep up @nan0s7 :1st_place_medal:

P.S : i know there are many generic named commits but i was lazy to clone my branch locally lol, i could also have create a pictures folder (and made picture link relative, but then we wouldnt be able to one click see it full size)

i'll definitely submit another proper PR in the future with clearer instructions, when i get a good enough feel of how this works, but the first experience is already great :+1:

nan0s7 commented 5 years ago

Hey, thanks for all the positive feedback! :D

I don't think I'll be accepting your pull request though; I'm not sure my script really benefits from having screenshots.

I have actually been thinking of renaming temp.sh but I've been undecided on a new name for ages. I was thinking of just removing update.sh because most people have their own way to update code, especially now that there's a version of it in the AUR now. Now that you mention it, I should probably add a command-line option to automatically run the script in background mode.

If you have any other suggestions, feel free to put them forward. I look forward to what you do in the future :)

wonderingabout commented 5 years ago

welcome :+1:

it's up to you to decide if you find screenshots helpful or not, personally i think they illustrate quite well but if you're fine without then its fine too :+1:

as for the .sh names yes they are really unintuitive, atm i'm using the foreground.sh because it reassures me to see that the script is running

but in the end maybe i'll switch to the background one after some time

i suggest to simply go for these names :

they are intuitive names, i read that background requires git but didnt try it yet, so maybe rename it backgroundgit.sh, rename it as you see fit

i'll submit a PR on clearer readme and comments when i dig into your nice tool enough :+1:

thanks again :)

nan0s7 commented 5 years ago

Yeah I doubt I'll change the name for the update.sh because it's, for me, intuitive. The script just happens to run a background process of temp.sh because I assumed the user would just have my script running in the background upon boot anyway. It depends on git because that's what it uses to download the new updates.

I was thinking of changing temp.sh to something like nfancurve.sh or something like that to indicate that it's what you should be using. I'm not sure yet, I need to think about it some more.

But yeah, look forward to hearing more from you!

No problem, I'm glad you're finding it useful :)