Comments (46)
As bash-fetishist, i'll do this, did this for other projects too. If i'll be first one to pass review, money can be donated to fsfe ;)
from vm.
@morph027 wrote:
As bash-fetishist, i'll do this, did this for other projects too. If i'll be first one to pass review, money can be donated to fsfe ;)
Rewrite is almost done (only 2 errors left afaik), just donated $149.71 to FSFE as @morph027 and me did everything ourselves. @PietsHost contributed with one commit but left the branch after that. Total commits will be around ~300. The code is improved very much and now Travis is included to be sure that it stays that way.
Thank you for your good work and ideas @morph027, and thanks @PietsHost for your commit.
from vm.
@ypid Started a PR here: #21 Feel free to help.
from vm.
I thought you pushed all the changes to the branch, haven't checked all the files in the pull req by view. So the entire /static still needs to be changed then? That will take a little while, no problem.
from vm.
The code works, but doesn't look good. It needs to be consistent and properly intendented to please the eye. Stuff like:
command
if [ $? -eq 1 ]; then ...
Could be replaced with the if argument
directly instead, like so:
if command; then ...
Also use elif
where possible to produce more code on fewer lines.
Except that the code is not consistent. In some places we use >
instead of -gt
which is the proper way of doing it.
And then we have... intendeation(!!) We should use 4 spaces in a if argument for example. This is a good example.
We also have this in many places:
echo -e "\e[32m"
read -p "Press any key to continue... " -n1 -s
echo -e "\e[0m"
Which could be done on the same line.
There are a lot of other stuff I can't think of now, but anyone who have some time left over could do this one step at the time. My suggestion is to start with the smaller scripts and then work our way up to the main scripts (installation & setup).
There is a personal bounty for the person/persons who does this in one sweep (everything at once), meaning if someone decides to spend time on this, create a PR and do everything according to my suggestions (or better) that person (or group of people) will be rewarded with $150 (total amount) on their PayPal account when I (together with maintainers of this repo) have approved the changes. Please ask if you have any questions regarding the reward/bounty.
I posted a project that you can use for this if you want, it's found here: https://github.com/nextcloud/vm/projects/1
Let's get this done! cc @nextcloud/vm
from vm.
List of scripts...
- ./nextcloud_update.sh
- ./nextcloud_install_production.sh
- ./static/ip.sh
- ./nextcloud-startup-script.sh
- ./static/trusted.sh
- ./static/instruction.sh
- ./static/ip2.sh
- ./static/ntpdate.sh
- ./static/security.sh
- ./static/passman.sh
- ./static/temporary-fix.sh
- ./static/collabora.sh
- ./static/history.sh
- ./static/spreedme.sh
- ./static/change-ncadmin-profile.sh
- ./static/nextant.sh
- ./static/phpmyadmin_install_ubuntu16.sh
- ./static/update.sh
- ./static/redis-server-ubuntu16.sh
- ./static/nextcloud.sh
- ./static/change-root-profile.sh
- ./static/setup_secure_permissions_nextcloud.sh
- ./static/change_mysql_pass.sh
- ./static/test_connection.sh
- ./static/adduser.sh
- ./lets-encrypt/test-new-config.sh
- ./lets-encrypt/activate-ssl.sh
from vm.
This is unclear to me:
sudo passwd "$UNIXUSER"
if [[ $? -gt 0 ]]
then
sudo passwd "$UNIXUSER"
else
sleep 2
fi
If it fails one time, it can fail a second time ;) If we really need to change the password, we should do this prompt in a loop until it was successful.
Same here:
sudo -u www-data php "$NCPATH/occ" user:resetpassword "$NCADMIN"
if [[ $? -gt 0 ]]
then
sudo -u www-data php "$NCPATH/occ" user:resetpassword "$NCADMIN"
else
sleep 2
fi
from vm.
I would like to use variables for colored messages.. like this:
green='\e[32m'
reset='\e[0m'
[.... some code ....]
printf $green"here's my text"$reset
instead of
echo -e "\e[32m"
read -p "Press any key to continue... " -n1 -s
echo -e "\e[0m"
and also use printf instead of "echo -e", as described in the APPLICATION USAGE section of the following spec, for reliably portable code: POSIX specification for echo
from vm.
@@PietsHost Also, you did some great stuff with prigress meters in you modified scripts. Would be nice to see them here as well. 👍
from vm.
Ok....according to my list, every script in my branch is shellcheck'ed at least...
from vm.
Ran the install on a fresh vm, no problems installing. Only the documents app...
from vm.
Great! Yeah, the documents app fails. Please comment it out in this #21 PR as well.
I will run the patch later and test that everything works out. Great job @ezraholm50!
from vm.
Btw, now @nextcloud/vm is available. FYI. :)
from vm.
@enoch85 Done! Better to have us both check it indeed. I'll run it another time somewhere tonight if i can manage to get some time off.
Oh wow nice one at the VM team!
from vm.
All the files in 'static' aren't checked yet... ;)
from vm.
Actually I think they are, I git cloned the entire ShellCheck branch, moved all the scripts to /var/scripts
And I ran this just to be sure.
sed -i 's|production repo link|shellcheck repo link|g' /var/scripts/*
So that to me feels like everything that has been changed works.
Or did you mean that you didn't had the chance to change them?
@enoch85
from vm.
@ezraholm50 What I mean is that you should upload your changes to Github, the spellcheck branch. Please check for yourself, the only change I can see is that you hashed out the Documents app. :S
from vm.
@ypid PR failed. We followed the instructions from ShellCheck. These scripts are built over time for the past 2 years now. We have tested everything thoroughly over time, but with ShellCheck it fails .
Feel free to open a PR if you want.
from vm.
ShellCheck failed? It just gives suggestions 😉 Maybe you can start small. Run Shellcheck and then check the description of each issue.
Each issue has an ID with which you can visit the wiki of Shellcheck:
Example: https://github.com/koalaman/shellcheck/wiki/SC2086
from vm.
We did, most of the suggestions work, though not all. And to be honest with about 10% I really wouldn't know what to change.
This is put on long term, @enoch85? My idea was also to start small with some double quote's and few other things that work.
Thanks for the suggestion anyway!
from vm.
@ypid @ezraholm50 This is opensource, anyone can contribute ;) PRs are more than welcome.
from vm.
Why did you close @ezraholm50?
from vm.
Ref #36
from vm.
@ezraholm50 I still don't get why you closed this? It's long term, and it doesn't hurt if someone else wants to take care of this - I reopen.
from vm.
@morph027 Thank you! If you find anything that could be improved even more let's do it. I can't remember all the thousands of lines right now. :)
And please ask me if you are wondering about any functions.
from vm.
Ok...let's start ;)
Why do we sometimes just ask to press any key to do something? What if the person does not want to take the action?
from vm.
Yes, that's a great example of old code that needs to be upgraded. A loop would be much better - the intention is to give the user another chance to type the correct password if it fails, and this was one of the first things I wrote some years ago.
So please, make it a loop until the attempt is successful. Thank you!
from vm.
Why do we sometimes just ask to press any key to do something? What if the person does not want to take the action?
This is becuase in some cases there are information that could be useful to the user, and if he/she wouldn't press a key to continue he/she wouldn't have a chance to read what happened. Like for example; it's good to know if something has failed. Do you have a better suggestion?
from vm.
This is becuase in some cases there are information that could be useful to the user, and if he/she wouldn't press a key to continue he/she wouldn't have a chance to read what happened. Do you have a better suggestion?
No, fine...just for better understanding. Just in case of multiple runs, the user probably does not want to adjust the timezone every time ;) But this could be fixed with some markers, which indicates, that a section succeeded and we can skip it.
from vm.
By the way...do you really want to review one HUUUUGE pull request? Just for reviewing purposes, it might be better to make small potions (e.g. for each script) to have atomic discussions in places, where it belongs.
from vm.
The script isn't intended to be run several times (nextcloud-startup-script.sh) and assumes certain things (don't remember everything now) but if we could make it safe to run several times that would be awesome. Like check if things are already setup and in that case skip them - 👍 💯 from me on that. In later coding I have thought of making it possible to run several times, so the code is kind of mixed with the old where it would fail hard if that happened.
Some things I remember from the top of my head;
- It set's a new mysql password and then deletes the password file in the phpmyadmin installation, on a second run that would fail hard. (Btw, make phpMyadmin installation optional and come up with a better way of fetching the needed password for the installation. The current password should always be in .my.cnf, don't remember why I kept mysql_password.txt..)
- The UTF8 convert isn't supposed to be run twice. Maybe check if that's already done and skip if it is..?
By the way...do you really want to review one HUUUUGE pull request?
As everything is tightly connected to eachother (some scripts depends on others etc, it's crucial that everything is perfect, and in a HUGE PR we could test that branch in one go on a test VM for example... So yes, I prefer a HUGE PR. :) I'm ofc counting with that you test all the scripts together and separately as well to be sure everything is bug free. :)
from vm.
@morph027 BTW, join #techandme on IRC if you want to discuss there instead. :)
Also, regarding the PRs, you could commit to your repos master and I can check it there before you make a PR out of it.
from vm.
Oh, and btw. On this was on my todo list as well, maybe you can check why it happens sometimes?
from vm.
to have atomic discussions
We could do that in IRC or in your repo.
from vm.
For reference: https://github.com/morph027/vm/tree/20-increase-code-quality
from vm.
Ah I've got another hint, bc I've just seen your variables:
All-caps environment variable names are actually reserved by POSIX-specified convention for variables with meaning to the shell or operating system, whereas lowercase names are reserved for application use. (Even if you aren't exporting your own variables to the environment, setting a shell variable overwrites any like-named environment variable, making the convention necessarily apply in both places)
from vm.
@PietsHost Hi there! :) So nice of you to contribute! :)
Right now we are working in @morph027 repo. Add you sugesstions there as a PR and we can discuss it.
Thank you!
from vm.
Okay :)
from vm.
@morph027 @PietsHost How far have we come with the scripts?
from vm.
No updates today, sorry.
I'm really busy right now.. maybe tomorrow
from vm.
Same here..did some minor stuff but will have some more time tomorrow ;)
from vm.
@morph027 OK, nice! Good job! 👍
Now, let's improve the code in general. Things like removing unnecessary lines, compress if arguments, add functions and improve how the code works. You had some smart stuff going on with your while loops, and I think they can be used in more places as we discussed earlier.
I'm on IRC if you want to discuss.
from vm.
All if arguments are checked. Left some comments for you @morph027 to improve even more. Also, please do a sed
on the file checking and dir checking. I did if ! [ -f ...
while it should be if [ ! -f ...
from vm.
https://github.com/morph027/vm/pull/13#issuecomment-291194697 @morph027
from vm.
Awesome work guys, thanks!
from vm.
❤️
Great....will still continue looking at the code if my brain has some spare time left ;)
from vm.
Related Issues (20)
- Update to Nextcloud 27.1.4 - error "sudo -u www-data php /var/www/nextcloud/occ maintenance:repair failed" HOT 2
- GUI's "Apps" page throws Internal Server Error HOT 3
- VM cannot be imported into Proxmox 8 HOT 1
- No bootable Medium at VirtualBox 7 HOT 9
- [Documentation]: Differences between "Integrated" and "Docker" installs for docserver are confusing/unclear HOT 4
- Can't start the first setup. HOT 2
- Your .ocdata are missing in the Nextcloud data folder. HOT 3
- OVA deployment in vsphere 8 fails with error HOT 12
- Boolean flag missing HOT 5
- Could not boot files_inotify: Could not resolve dispatcher! Class "dispatcher" does not exist - new since recent NC update HOT 1
- Unable to update Nextcloud instance su HOT 4
- Restoring the VM on Proxmox VE 8.1.4 HOT 2
- Spamhaus Script Update URLS HOT 1
- Something got wrong during pihole-install.sh HOT 3
- Executable files are not executable (`+x`) after upgrade HOT 13
- Is it possible to bake traefik into your setup to have Nextcloud behind it? HOT 3
- Installation fails - clean ubuntu install HOT 31
- Error installing nextcloud HOT 4
- Adminer css url changed HOT 2
- Netdata installation fails HOT 6
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from vm.