Wow, @cristovaov, this is really fantastic. Thanks!

I've been thinking for a while about doing exactly this in some fashion. I like the idea of having areas broken up into functions. It definitely helps readability and should help when troubleshooting or extending specific areas as well. I also had no idea that shellcheck existed, which seems great on the surface.

I'm going to take a closer look at this soon and do some proper testing. Overall it seems to be on the right path already.

@jeremyfelt great, this makes me happy :)

I tried to follow the provision flow of the current script as close as possible so tests should be similar.
You can see along commits that I had used variables too but decided to move this to another branch on my fork. I was eager to submit this one.

Shellcheck is a nifty tool, the recommendations/guidelines you get are mint.
I couldn't make out wether the changes also speed up provision time, but my perception makes me feel it does. And if it actually does, it would be very minimal I suppose.

Thanks for the feedback! ~

@jeremyfelt Hi Jeremy, as I updated my fork I've cleaned this pr to one commit. The provision script has been tidied up too. Cheers~

I really love this.
We should test and merge this fast. Any further changes to provision.sh will make it harder for @cristovaov to keep this PR current.

For example: GitHub token (due to bad differences between master and develop branches)
@jeremyfelt we should merge master into develop + develop into feature/phpbrew (regularly) to keep everything in line. give me the go ahead and I will do it.

Maybe we can get this in within a week?! So I can update #611 for merge.

@cfoellmann Thank you! For now I am tracking any changes to provision.sh and update mine accordingly. It's good excercise ;)
On a side note: I was wanting to write this week proposing that I would do the same for the master branch. It would prevent conflicts when merging Develop into Master if this PR gets accepted.
How does this sound?

I will do some testing now.
It is "just" a rewrite and will not break anything in an existing vvv box so if everything checks out we should consider merging this in master directly.

This would make it easier for you. No need to waste your time on keeping track of the changes in master.

we should merge master into develop + develop into feature/phpbrew (regularly) to keep everything in line.

@cfoellmann - We should get to a point where we're merging develop into master more frequently. Though any time that does happen (beyond some recent accidents 😁), it should correspond with a release.

I definitely think we should get this PR into develop soon. At that point we should work on testing upgrades from the current master to the current develop so that we can confidently merge and call it 1.3.0. Ideally we have quicker incremental updates, but you know... :)

On a side note: I was wanting to write this week proposing that I would do the same for the master branch. It would prevent conflicts when merging Develop into Master if this PR gets accepted.

@cristovaov - I don't think we'll have much trouble merging develop into master, so no other version of the PR is necessary. If we do run into issues, I'll take ownership of resolving those conflicts. Thanks though!

I tested this as well yesterday and was really happy with things. Everything went smoothly on a fresh box and I haven't noticed any issues yet. A few notes from my end:

• A few comments got rearranged (see to of provision.sh and the apt_package_check_list package comments). I'd prefer if most of those stayed the same for the moment. If it does make sense to change a particular comment, it would be nice to see it on another commit.
• Comments describing a function should go just above the function name. At some point it could be cool to follow Google's shell style guide for function comments and full develop parameters, returns, globals, etc... but I think that can also wait to a future PR.
• The network_check function is used several times to check the network but doesn't seem to bail on the process entirely if a bad network is detected. The initial check should result in most other things not being called if it fails. This needs to match existing behavior.

@jeremyfelt I am with you on aiming for 1.3. On the other hand you misunderstood me. It meant to "rebase" develop on master to get everything from the stable branch into the unstable.

@cristovaov integrate 698f4c7 into your PR, please.

@cfoellmann Ahh, excellent - I did misunderstand. I'll work on that today. There were a few accidental merges to master on my end. :)

@jeremyfelt Noted and I agree with them. Will bring back the original comments. For the comment formatting, I'd like to follow the suggestion of doing this in a future PR and stick to the original formatting in this PR. For the networck_check function I think I understand what you mean, I think what is missing in the function is an exit 0to break it down.

@cfoellmann I'll go through the provisioning script at 9162a56

I'll ping you for review when I'm done. Thank you for the positive feedback!

@jeremyfelt @cfoellmann ping ;)

1st test:
Fresh install (loaded the zip https://github.com/cristovaov/VVV/archive/feature/rewrite.zip )

Results

==> default: Configuring WordPress develop...
==> default: Installing WordPress develop...
==> default: Running npm install for the first time, this may take several minutes...
==> default: Initializing grunt in WordPress develop... This may take a few moments.
==> default: grunt-cli: The grunt command line interface. (v0.1.13)
==> default: Fatal error: Unable to find local grunt.
==> default: If you're seeing this message, either a Gruntfile wasn't found or grunt
==> default: hasn't been installed locally to your project. For more information about
==> default: installing and configuring grunt, please see the Getting Started guide:
==> default: http://gruntjs.com/getting-started
==> default:
==> default: VVV custom site import
==> default: Cleaning the virtual machine's /etc/hosts file...
==> default: Adding domains to the virtual machine's /etc/hosts file...
==> default:  * Added vvv.dev from /srv/www/vvv-hosts
==> default:  * Added local.wordpress.dev from /srv/www/vvv-hosts
==> default:  * Added local.wordpress-trunk.dev from /srv/www/vvv-hosts
==> default:  * Added src.wordpress-develop.dev from /srv/www/vvv-hosts
==> default:  * Added build.wordpress-develop.dev from /srv/www/vvv-hosts
==> default: -----------------------------
==> default: Provisioning complete in 1131 seconds
==> default: For further setup instructions, visit http://vvv.dev
==> default: Running provisioner: shell...
    default: Running: inline script
==> default: mysql stop/waiting
==> default: mysql start/running, process 29034
==> default: Running provisioner: shell...
    default: Running: inline script
==> default:  * Restarting nginx nginx
==> default:    ...done.

Everything works out nice. the grunt build process is broken for me a long time now. So nothing new here.

@cfoellmann

the grunt build process is broken for me a long time now

I actually have some thoughts about that. I can open an issue for this if you'd like.
Looking forward the further feedback~

If you have the same problem and even an idea to fix it open a dedicated issue.

I updated my old vvv with the new provision.sh and it works nicely.

updated with latest changes in the provisioning script. any ETA on the merge?

Ok, I provisioned with this from scratch and used it for about a week. I'm going to merge it into develop so that we can get some more real world use. I may go through and re-organize just a bit post merge and we'll see if we run into any bugs. I haven't fully tested the network detection, and I'm not sure if I understand yet how it would work compared to the current version, but let's go for it. :)

Thanks @cristovaov!

@jeremyfelt
This great, you're welcome and thank you very much for the merge.
Obviously, the script is open to any changes necessary. I am curious of any real world use feedback too. How did you find it?

From memory, network detection will now exit the script straight away rather than continue until the end of it. A comparison side by side of the functions could perhaps help.

Would it be an idea for me to write a page in VVV's wiki about this provisioning script?
I was thinking of something in the likes of an index of the functions and an example use.
I could work on it this week.