Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some suggestions for your code #58

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

MarkusLange
Copy link

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:

  • allways use of the latest ruTorrent
  • no need to care about the distro version
  • remove unnecessary stuff
  • add support for arm* systems (scgi)
  • correcting external ipv4
  • deactivate not supportet plugins
  • redirect http to https
  • correct terminal colors
  • fix changelog and todo view
  • and a little bit there and there

Take a look and grep what you like

BR
Markus

cleaning and sorting the code for a better plain view, make FASTMODE standard, and download Changelog and TODO to show them if selected on the menu, also remove all files after quit
correkt some leftover misc after cleaning
remove FASTMODE from source
move some lines around
Clean code
auto use of latest ruTorrent
auto install scgi from external source if needed
made script debian-/derivate indipendent
remove plugin handling
correcting external IP to IPV4
clean code
remove demo-mode stuff
remove display help
remove distro detection
remove not needed ports.conf
add https redirection
correct colors
make webuser password look nice
add armel support
put deactivate plugins routine in loop
remove unused variables
remove change http port, not needed since http to https redirection
edit rtorrent port-range function to show actuall range and add save by empthy range
shift PREPARE_CONFIG_FILES around to repair Change rTorrent Port-Range
show TODO and Changelog view correct for terminal
show port range correktly inside MENU
some code cleaning
shifting some lines
@Bercik1337
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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 :)

add a discription what comes next
expand some "----" lines
rename apache rutorrent .conf
add some HEADER lines
remove SET_HTTP_PORT complete
@MarkusLange
Copy link
Author

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

expand the comment for thr Plugin deactivation and add geoip
add some color inside INSTALL_COMPLETE window
lengthen some lines
make creating self-signed certificate look nice
shifting some lines around
move a2enmod ssll & rewrite
reintroduce os detection with an smaller footprint
correct space to tab
@MarkusLange
Copy link
Author

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

fix to match exact match by detect os
    * allways use of the latest ruTorrent
    * no need to care about the distro version
    * remove unnecessary stuff
    * add support for arm* systems (scgi)
    * correcting external ipv4
    * deactivate not supportet plugins
    * redirect http to https
    * correct terminal colors
    * add autodl-irssi plugins
    * update .rtorrent.rc to the new commands
    * fix changelog and todo view
    * and a little bit there and there
a little bit of shifting around
* put functions together for more order
* make pre-installation packages fully silent
* correct autoredirect from http to https
	* now choose between apache2, nginx and lighttpd as webserver (Rt-Install-minimal-apache2_ngnix_lighttpd.bash)
	* create htaccess passwords now with openssl
	* remove ToDo-List from the Menu
rewrite to user rpc.socket
add ruTorrent update
move rtorrent user and rutorrent user input to the menu
cleaning the code
Remove function WAIT_A_MINUTE

shifting a bit around
fix RSS Plugin install missing php-xml for dom
deactivate Plugin dump
* adopt the actual ruTorrent branches (v4 & v5) since autodl is
  brocken in v5
* transfer naming from Bercik to me for less confusion
* redirect Todo and Changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants