search

Plippo / asus-wmi-screenpad

Variation of the asus-wmi kernel module with screenpad brightness support
Other
155 stars 19 forks source link

Added install.sh #53

Open Fir3Storm31 opened 1 year ago

Fir3Storm31 commented 1 year ago

Added install.sh to make the installation easier and incorporated the option to patch the module. Updated the documentation.

This is a further implementation of another pull-request: #45 .

Plippo commented 1 year ago

Thank you for creating the pull request. I like the incorporation of prepare_for_current_kernel.sh directly into the script, but to be honest, I don't really see the need for the -p flag, as patching is virtually always needed to build the correct version for the current kernel. Do you see any use cases where patching wouldn't be needed?

Fir3Storm31 commented 1 year ago

You're probably right, I always automatically give options, but in this case, it is not needed.

Plippo commented 1 year ago

Great, thanks for the change, I think it's much better now. But what I noticed is that in the readme, the "manual" method still mentions prepare_for_current_kernel.sh, which is deleted in this PR. Maybe we should keep the file for now? Or, as an alternative, we could add an option to install.sh that makes it only download and patch the files without installing (just as prepare_for_current_kernel.sh did) and mention that in the readme instead. Both would be fine with me.

Fir3Storm31 commented 1 year ago

We can use the flag -i to install and the flag -p to patch, with no flags it would just install the packages dkms and patch. Is it fine for you?

Plippo commented 1 year ago

Well as it is called install.sh, I think when called without flags it should still install everything. I don't think we need a call to only install dkms and patch. -p to patch sounds good.