dominiksalvet / asus-fan-control

Fan control for ASUS devices running Linux
MIT License
320 stars 37 forks source link

Merge systemd .service files #17

Closed agura-lex closed 4 years ago

agura-lex commented 4 years ago

Description There's really no need to have them as separate units

Additional context Discussed in https://github.com/dominiksalvet/asus-fan-control/pull/16

agura-lex commented 4 years ago

Couldn't push it to master even though I'm in CODEOWNERS for these files and can't add myself to reviewers as well.

dominiksalvet commented 4 years ago

Sorry for the delay. I will get to it tomorrow.

dominiksalvet commented 4 years ago

All right, so finally I have found some time to sort everything out.

The truth is that I have set branch protection for the master branch (as almost in all of my projects) and initially I have completely misunderstood its purpose. It particularly affects collaborators and prevents them from making possible fatal changes to the repository. One of that possible changes is a direct push to the master branch without previous reviews and status checks. So that is the reason you could not directly push to the master branch.

CODEOWNERS has no connection with the mechanism described above. It exists for automatic pull review requests. If a new PR is created, then reviewers are automatically chosen based on the affected files (and their owners stated in CODEOWNERS). It looks like GitHub does not allow to review your own pull request and it makes sense if you think about it...

After some thinking, I will keep the branch protection untouched. The only branch affected of my asus-fan-control repository is the master branch, so you should have no problems with creating and maintaining other branches in case you want to have them in this repository rather than in your fork. I will also try to be as quick as possible in case of further PRs with a few small changes like this one.

PR review will follow in a moment.

agura-lex commented 4 years ago

Also, does it run before the user logins just like now? I mean whether it will be applied when I am on the Gnome lock screen of Ubuntu 18.04.

Somehow I missed that part and didn't reply to it. Yes, it does. It's a system unit, so it's run before the user logs in: graphical login is available after graphical.target is reached, which is activated after multi-user.target. Here's a simple doc on systemd's bootup process.