Bercik1337 / rt-auto-install

rtorrent and rutorrent automatic installer for modern operating systems on seedbox
150 stars 27 forks source link

Some suggestions for your code #58

Open MarkusLange opened 11 months ago

MarkusLange commented 11 months ago

Hi Bercik,

nice that you take the work of Kerwood future, I have overlooked it and want to make you some suggestions for the code for you to you service a little bit easier:

Take a look and grep what you like

BR Markus

Bercik1337 commented 11 months ago

I see you desperately want to remove FASTMODE everywhere. Why? It's very useful for r&d stuff to quickly see if script is working properly. Leaving crossed off unsupported systems is also a good thing. People tend to use EOL system and then whine it's not working. Now it's clear for them - it won't fly

Same goes for DEMO mode. When I shoot video, it saves me a lot of time in editing afterwards.

Arm scgi section looks nice, and disabling plugins section as well.

Extra hashes in Heavy I/O configuration are there for a reason.

It seems your PR is work of many people. Once "FAIL" is renamed to "fail" and others "fail" to "failed".

All and all, I think I will pull in few lines. Probably in 2 weeks.

MarkusLange commented 11 months ago

I see you desperately want to remove FASTMODE everywhere. Why? It's very useful for r&d stuff to quickly see if script is working properly.

For this I have changed clear to clear -x so you get a new screen but everythink is visible above with scrolling Another point is, the script takes some time so why punish the user with more time to wait.

Leaving crossed off unsupported systems is also a good thing. People tend to use EOL system and then whine it's not working. Now it's clear for them - it won't fly

Maybe who is the one to judge, I get it working on Debian 9 without any problems, so EOL is relativ, and every time there is a new derivate version or one hits EOL the script need to be altered

Same goes for DEMO mode. When I shoot video, it saves me a lot of time in editing afterwards.

So this case is a usecase for you, for parts like this screenshorts or videos I would prefer editing this script for the special needs not to integrate it in a version for everyone

Arm scgi section looks nice, and disabling plugins section as well.

Extra hashes in Heavy I/O configuration are there for a reason.

if you mean the Rutracker_check Plugin this is not working based on the folder rights since home moved to 750, others, like www-date are prevent to use it Or did you mean the rtorrent.rc file I changed nothing I just remove some # in the lines to make it look more even

It seems your PR is work of many people. Once "FAIL" is renamed to "fail" and others "fail" to "failed".

May the many have the same ideas then maybe, I renamed it because of reasons, everything is lowcase why this not?

All and all, I think I will pull in few lines. Probably in 2 weeks.

Bercik1337 commented 11 months ago

Arm scgi section looks nice, and disabling plugins section as well. Extra hashes in Heavy I/O configuration are there for a reason.

if you mean the Rutracker_check Plugin this is not working based on the folder rights since home moved to 750, others, like www-date are prevent to use it Or did you mean the rtorrent.rc file I changed nothing I just remove some # in the lines to make it look more even

Sorry maybe we misunderstood each other. Heavy I/O is placed in rtorrent.rc. Reasoning for extra hashes is: this is "extra" feature, not a must-have for everyone. I wanted to "separate" it somehow from the rest of the code. It actually surprised me there are people with 1Gbit+ pipe who use my script. This section is for them :)

As for scgi and plugin disable, it's great! I'm definitely going to pull that into my script. 10/10

This will be a challenge for me, because I only had one PR in my life, and this time - I would like to dissect some lines from your proposal. Because some I will surely pull in, and other I would stop. So just letting you know, it will take time :)

MarkusLange commented 11 months ago

Arm scgi section looks nice, and disabling plugins section as well. Extra hashes in Heavy I/O configuration are there for a reason.

if you mean the Rutracker_check Plugin this is not working based on the folder rights since home moved to 750, others, like www-date are prevent to use it Or did you mean the rtorrent.rc file I changed nothing I just remove some # in the lines to make it look more even

Sorry maybe we misunderstood each other. Heavy I/O is placed in rtorrent.rc. Reasoning for extra hashes is: this is "extra" feature, not a must-have for everyone. I wanted to "separate" it somehow from the rest of the code. It actually surprised me there are people with 1Gbit+ pipe who use my script. This section is for them :)

As for scgi and plugin disable, it's great! I'm definitely going to pull that into my script. 10/10

This will be a challenge for me, because I only had one PR in my life, and this time - I would like to dissect some lines from your proposal. Because some I will surely pull in, and other I would stop. So just letting you know, it will take time :)

Use what you like, and take your time, no rush, and if you have questions about my code what it did exactly ask

MarkusLange commented 10 months ago

Bercik I add a different version of DETECTOS with a smaller footprint so there a less lines to change if any release or end of life is happen