Skip to content

Conversation

@webflo
Copy link
Member

@webflo webflo commented May 24, 2016

Follow-up for #37

I tried to fix the async via guzzle in 69038ac#diff-972a789eb631b4e9b19070bc31cbe900L36 and was working in the test. It took lets than 4 seconds to fetch all files. But i hit composer/composer#4764 if i run it as composer plugin :/

Refactored everything and used composers RemoteFileSystem to get the job done. Documentation is not up to date yet.

@webflo webflo changed the title Fetch files via cgit.drupalcode.org directly [WIP] Fetch files via cgit.drupalcode.org directly May 24, 2016
@webflo
Copy link
Member Author

webflo commented May 24, 2016

It should be faster (#35) and work on Windows (#11)

@greg-1-anderson
Copy link
Collaborator

Haven't tried it yet, but read through the changes and it looks fantastic. Great job.

@webflo
Copy link
Member Author

webflo commented May 24, 2016

@greg-1-anderson Why is expected that the index.php is not present after the initial composer install? Both packages (the scaffold and drupal core) are in the composer.json. I can't spot the difference to the previous implementation.

@greg-1-anderson
Copy link
Collaborator

I just ran phpunit locally, and the tests passed. I'm going to have to scratch my head a bit about why the failure is happening on Travis; the cause is not immediately apparent to me.

@greg-1-anderson
Copy link
Collaborator

The only difference with the previous implementation was that the robo task was exec'ed, whereas now the files are fetched directly from the composer plugin process. However, I cannot reproduce this locally, so it's hard for me to imaging what it is about the Travis system that causes the test to fail there.

Does a local phpunit run pass for you? I presume that it probably does.

@webflo
Copy link
Member Author

webflo commented May 25, 2016

#40 failed after composer update. I change the test to match the new behaviour. It makes more sense anyway. Thanks @greg-1-anderson for your help.

@greg-1-anderson
Copy link
Collaborator

LGTM. Some minor changes to drupal-composer/drupal-project will be needed once this is merged.

@webflo
Copy link
Member Author

webflo commented May 25, 2016

@greg-1-anderson Yes the version constraint. What else?

This was referenced May 25, 2016
@webflo webflo changed the title [WIP] Fetch files via cgit.drupalcode.org directly Fetch files via cgit.drupalcode.org directly May 25, 2016
@webflo webflo merged commit ad5fb54 into drupal-composer:master May 25, 2016
@webflo webflo deleted the composer-rfs branch May 25, 2016 17:52
@greg-1-anderson
Copy link
Collaborator

I added a PR, drupal-composer/drupal-project#155 to demonstrate. I think it's about right, but have not tried it.

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