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

Issue #38: decouple drupal download from drupal install #49

Closed
wants to merge 2 commits into from

Conversation

mradcliffe
Copy link

I might as well open this for review so that I can make changes or figure out what tests are needed.

I just rebased, but for some reason had to force push to my remote.

# Create database and install Drupal.
mysql -e "create database $DRUPAL_TI_DB"

cd drupal
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

cd "$DRUPAL_TI_DRUPAL_BASE"

@mradcliffe
Copy link
Author

I have not had a chance to work on this lately. Sorry.

@LionsAd LionsAd modified the milestone: 1.5.0 Dec 12, 2015
@yanniboi
Copy link
Contributor

Hello @mradcliffe and @LionsAd, I have had a chance to look at this however the first thing I did was rebase issue-38 on master which needed some merge conflicts solving, so I am not sure what the best way to contribute to this PR is.

I can create a PR into mradcliffe:issue-38 but that will be messy to review, so I have also created a PR from yanniboi:issue-38 to yanniboi:master (matching LionsAd:master) on my fork which makes the overall review and testing easier (maybe...) here: yanniboi#1

What I have mainly done is, rather than changing the behat and simpletest runners to use a different function to install drupal, I have made it so 'drupal_ti_ensure_drupal' still downloads and installs drupal, but it downloads drupal by calling another re-entrant function 'drupal_ti_ensure_drupal_download' which can be called by phpunit-core rather than 'drupal_ti_ensure_drupal'. This means no changes are needed to behat and simpletest.

Let me know what the best way to get this into the current PR is or if it is easier to create a new PR?

@yanniboi
Copy link
Contributor

I have spoken to @mradcliffe on IRC and he is happy for me to start a new PR from my fork. Will post a link to the new PR here.

@yanniboi yanniboi mentioned this pull request Jan 23, 2017
@mradcliffe
Copy link
Author

Closing in favor of other pull request.

@mradcliffe mradcliffe closed this Jan 23, 2017
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.

3 participants