drud / quicksprint

A basic toolkit to get people started with ddev and a Drupal codebase on macOS, Linux, or WIndows
Other
24 stars 16 forks source link

Fixes for v0.0.6 release #32

Closed BrianGilbert closed 6 years ago

BrianGilbert commented 6 years ago

[#21] Ensure ddev is added to path, Let the user know we exited if they didn’t hit Y/y, add that install should be run via terminal on mac and Linux. [#26] Added error checking to all shell scripts. [#28] add release version to archive filename. [#30] Don’t install docker by default, improve messaging about Docker memory requirement. [#31] Add version number to Docker installers, remove spaces from filenames, make packaging of installers optional. Don’t automatically install docker on mac, make the user do it manually instead. Improve visibility of requirement to increase docker memory limit. [#30 #31] When running packaging script, delete and build it clean if staging dir already exists.

rfay commented 6 years ago
####
# Shall we package docker installers for mac and windows with the archive?
# !!You don't need to hit enter!!.
####

I apparently do need to hit something :) Maybe Y/n prompt? What's the intent for a normal build? No docker?

Hitting "y" worked for me the first time. The second time I did it (after validating the downloaded versions) I didn't want to bother, so hit "n", and nothing happened. Despite the instructions I also hit "" and also hit <n><\n>. Turned out that one of those worked, but it gave no feedback at all so I was really wondering, Eventually it showed download results while I was writing this.

rfay commented 6 years ago

/Users/rfay/go/src/github.com/drud/quicksprint/package_additions.sh: line 9: xz: command not found

I'm doing a brew install xz now, and very few users will ever run this script. However, it might want to test for the existence if necessary tools at the beginning instead of at the end. command -v might do well, https://stackoverflow.com/a/677212/215713

rfay commented 6 years ago

At the end of the script it displays everything in the drupal_sprint_package staging dir, which doesn't seem very useful. It's a result of printf "${GREEN}The sprint tarballs and zipballs are in $(ls $STAGING_DIR_BASE/drupal_sprint_package*).${RESET}\n" - How about we change that ls to something more specific so it will only show the tarball and zipball?

Also, I note that v0.0.6 seems to be wired into the QUICKSPRINT_RELEASE. It would be loads better to get it from git. Let's make QUICKSPRINT_RELEASE=$(git describe --tags --always --dirty) - that will get the current tag in there, or if it's not on a tag, something like the nice tag of this PR currently, v0.0.5-12-g70fbcaf

The sprint tarballs and zipballs are in /Users/rfay/tmp/drupal_sprint_packagev0.0.6.tar.gz
/Users/rfay/tmp/drupal_sprint_packagev0.0.6.zip

/Users/rfay/tmp/drupal_sprint_package:
COPYING
SPRINTUSER_README.md
bin
ddev_tarballs
install_ddev.cmd
install_ddev.sh
licenses
sprint.tar.xz
start_sprint.cmd
start_sprint.sh.
rfay commented 6 years ago
STAGING_DIR_NAME=drupal_sprint_package
STAGING_DIR_BASE=~/tmp
STAGING_DIR=$STAGING_DIR_BASE/$STAGING_DIR_NAME

These are all currently set up so the staging directory and the contents of the tarball are the same, so if I untar the sprint package to test it, it overwrites the staging directory (or perhaps would have things in there it shouldn't). Could we 1) Change the name of the tarball to something else? or 2) Change the name of the staging directory? Maybe both?

rfay commented 6 years ago

Let's make the xz unpack of the drupal repo quiet (in start_sprint.sh/cmd) instead of showing all that huge extraction of the .git directory and everything. That's pretty foreign to most people.

rfay commented 6 years ago

start_clean.sh doesn't start the ddev site. Shouldn't it? At the end of the the script it errors? out with Project is not currently running. Try 'ddev start'.

I think it doesn't work.

I'm not sure the "clean" idea is meaningful. What is "clean" supposed to be? Blow away everything?

I am able to ddev start and get to the install screen. However, it doesn't have a default db loaded, probably you want them to go through the install process?

rfay commented 6 years ago

Oh but I have only tested on Mac so far. I will try windows and Linux this weekend.

rickmanelius commented 6 years ago

@rfay So based on your last comments... pull this and then submit additional PRs after? I would rather adopt that approach before my next round of testing. I'm on a fiber connection, so I can push a release for others to start working against for next round.

BrianGilbert commented 6 years ago

@rickmanelius that will make it hard to track these comments, unless new issues are created

rickmanelius commented 6 years ago

@BrianGilbert Fair enough. I'll work against this branch and rebuild/test locally.

rickmanelius commented 6 years ago

Testing

Performed by Rick Manelius as of 2018-04-06

macOS

Hardware Setup

I’m using a laptop with the following high-level specifications:

Software Setup

Preparation

Cleanup

Artifacts

Using as End-user

Recommendations

rickmanelius commented 6 years ago

Open items/questions:

rickmanelius commented 6 years ago

Testing so far, I can now get the zips generated. Moving on to testing as a macOS user. About to test as an end-user after I remove Docker for Mac and ddev. I am noting the one dependency of having docker installed to make the build, but that shouldn't trip too many people up.

rickmanelius commented 6 years ago

Finished and updated my review for macOS. Overall, the language and guidance between steps is much better then my previous pass https://github.com/drud/quicksprint/issues/21#issuecomment-378830583. I was also successful, which is great! I believe a few people have mentioned testing Linux/Windows to confirm the ability to get to a success.

FWIW, I put a few other recommendations for either messaging (when something is happening that takes a while) or potentially re-ordering some things (such as needing to run ddev start before running the cleanup script). That said, the most important thing is ensuring we have cross OS success, and other nice upgrades are just icing on the cake.

BrianGilbert commented 6 years ago

@rickmanelius I added a check that docker is running in to the relevant scripts in https://github.com/drud/quicksprint/pull/32/commits/3be763e59e1d7128fea986b418b30abfeec16816 & https://github.com/drud/quicksprint/pull/32/commits/91e5690c22c078cad34176622adbedaa4aada88c, which I think I committed after you had pulled to test

geerlingguy commented 6 years ago

Testing this PR on Windows 10 Pro today... as soon as my kids will take a nap :P — I'll update here with any notes:

screen shot 2018-04-07 at 3 27 25 pm
BrianGilbert commented 6 years ago

@geerlingguy something went wrong if you had that many files.. it should only be 31 files in 5 folders when extracting the drupal_sprint_package.zip file.

geerlingguy commented 6 years ago

@BrianGilbert - I downloaded the release as instructed from https://github.com/drud/quicksprint/releases (drupal_sprint_package.zip). It was 1.34 GB and seemed to have all those files inside once expanded...

BrianGilbert commented 6 years ago

@geerlingguy There are heaps of fixes in this PR that aren't in the latest downloadable release. Also the last release was packaged by @rickmanelius and the package archives seem to have some weird issues/corruptions.. I've removed them now.

geerlingguy commented 6 years ago

@BrianGilbert - Thanks; hopefully we can get this merged ASAP and get a 0.0.6 so the release packages are updated :)

I've completed the rest of my notes from the Windows 10 install process. If I can, I'll try doing it again sometime before I head out for DrupalCon if there's a newer release (no way I lug the Thinkpad with me all the way to Nashville :P ).

BrianGilbert commented 6 years ago

@geerlingguy I think I've addressed all of your points.. either already in this PR, or just now as addirtions to this PR.

PS: You don't need to know the DB details if using PHPMyAdmin thats packaged with ddev.

geerlingguy commented 6 years ago

@BrianGilbert - Is the start_clean.cmd supposed to also end up installing Drupal? I needed the details because when I loaded the sprint project URL it took me to the Drupal installer.

BrianGilbert commented 6 years ago

@geerlingguy the start_clean.cmd script was missing a few steps, your feedback pointed me there so thanks.. I've added them in now..

geerlingguy commented 6 years ago

@BrianGilbert - Awesome! I'm not leaving until Monday around noon, so if there's a new release between now and then I can download to the Windows machine based on this PR's work, I'll re-test and verify things are working more reliably :)

BrianGilbert commented 6 years ago

@geerlingguy I'm going to make one now.. hopefully I can upload the artifacts without it timing out this time.. (crappy australian broadband)