search

neothematrix / noip-renew

Auto renew (confirm) noip.com free hosts
https://hub.docker.com/r/moebiuss/noip-renew
Apache License 2.0
54 stars 20 forks source link

Remove redundant $USER input #19

Closed benyjr closed 2 years ago

benyjr commented 2 years ago

I actually don't understand the point of: $USER $INSTEXE $LOGDIR

The addition of $USER is redundant in lines 7 and 8. I say this because the code does not allow this command to be inserted into a second user's crontab to be run as the first user.

First, $SUDO crontab -u $USER -l is creating the crontab entry in $USER's crontab. This intrinsically means that the commands in the crontab will be run as $USER. So why are we doubling down and saying that the $USER must run the command as $USER?

Angel0ffDeath commented 2 years ago

@benyjr I fully agree with you. Probably on some systems you need this, but on Raspbian you definitely don't need this and if you use this syntax even cronjob will not work, since it will try to start $USER.... In debian cronjob is: min hour day month dayofweek (what to do) [optional - Log]

The syntax: min hour day month dayofweek user (what to do) [optional - Log] is incorrect!!!

Angel0ffDeath commented 2 years ago

@neothematrix Seems long time you are not here... You can commit this change. Tested.

neothematrix commented 2 years ago

right, sorry for the late action, I also couldn't see why it's duplicated, thanks for th PR!