Add preFileDownload event on packages.json fetch #2434

Merged
merged 1 commit into from Nov 22, 2013

Conversation

Projects
None yet
8 participants
@JJK801
Contributor

JJK801 commented Nov 19, 2013

Hi,

In order to extend @naderman's work on pluggin system, this add a PreFileDownloadEvent dispatch when composer fetch packages.json (For ComposerRepository class).

It's usefull for us, because we need to fetch it using s3:// scheme.

Thanks,

Jérémy

private $rawData;
private $minimalPackages;
private $degradedMode = false;
private $rootData;
- public function __construct(array $repoConfig, IOInterface $io, Config $config)
+ public function __construct(array $repoConfig, IOInterface $io, Config $config, EventDispatcher $eventDispatcher = null)

This comment has been minimized.

@stof

stof Nov 19, 2013

Contributor

is there a use case for making it nullable ?

@stof

stof Nov 19, 2013

Contributor

is there a use case for making it nullable ?

This comment has been minimized.

@JJK801

JJK801 Nov 19, 2013

Contributor

not really, just following the existing ;)

@JJK801

JJK801 Nov 19, 2013

Contributor

not really, just following the existing ;)

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Nov 19, 2013

Contributor

I see a drawback in this PR: it introduces some cyclic dependency graph: Composer depends on RepositoryManager which depends on EventDispatcher, which depends on Composer. This is not really clean

Contributor

stof commented Nov 19, 2013

I see a drawback in this PR: it introduces some cyclic dependency graph: Composer depends on RepositoryManager which depends on EventDispatcher, which depends on Composer. This is not really clean

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Nov 19, 2013

Contributor

I just reorganized this code, in order to make possible to inject EventManager into RepositoryManager, but i saw it and your right, it's not really clean.

Contributor

JJK801 commented Nov 19, 2013

I just reorganized this code, in order to make possible to inject EventManager into RepositoryManager, but i saw it and your right, it's not really clean.

@JJK801 JJK801 referenced this pull request in naderman/composer-aws Nov 19, 2013

Merged

Add repositories (packages.json) management using S3 #5

@omansour

This comment has been minimized.

Show comment
Hide comment
@omansour

omansour Nov 19, 2013

👍 will allow us to secure our packages.json file on our instance of satis

👍 will allow us to secure our packages.json file on our instance of satis

@Seldaek

This comment has been minimized.

Show comment
Hide comment
@Seldaek

Seldaek Nov 20, 2013

Member

@stof it's not great but I'm not sure there is a way around this.

Member

Seldaek commented Nov 20, 2013

@stof it's not great but I'm not sure there is a way around this.

Seldaek added a commit that referenced this pull request Nov 22, 2013

Merge pull request #2434 from JJK801/extend-pre-file-download
Add preFileDownload event on packages.json fetch

@Seldaek Seldaek merged commit 1ee30ea into composer:master Nov 22, 2013

1 check passed

default The Travis CI build passed
Details
@NETZkultur

This comment has been minimized.

Show comment
Hide comment
@NETZkultur

NETZkultur Nov 22, 2013

Hi,

after a composer self-update i get the following error

[ErrorException]
Argument 4 passed to Composer\Repository\VcsRepository::__construct() must be of the type array, object given, called in phar:///usr/local/bin/composer.phar/src/Composer/Repository/RepositoryManager.php on line 104 and defined

it seems that the constructor arguments aren't compatible anymore...

Hi,

after a composer self-update i get the following error

[ErrorException]
Argument 4 passed to Composer\Repository\VcsRepository::__construct() must be of the type array, object given, called in phar:///usr/local/bin/composer.phar/src/Composer/Repository/RepositoryManager.php on line 104 and defined

it seems that the constructor arguments aren't compatible anymore...

@Stelian

This comment has been minimized.

Show comment
Hide comment
@Stelian

Stelian Nov 22, 2013

Breaks entirely: #2444

Stelian commented Nov 22, 2013

Breaks entirely: #2444

@fadoe

This comment has been minimized.

Show comment
Hide comment
@fadoe

fadoe Nov 22, 2013

(👎) Same problem here.

fadoe commented Nov 22, 2013

(👎) Same problem here.

@robertfausk

This comment has been minimized.

Show comment
Hide comment

👎 :-(

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Nov 22, 2013

Contributor

Why does VcsRepository break standard repository composition? is this argument ($driver) really used?

Contributor

JJK801 commented Nov 22, 2013

Why does VcsRepository break standard repository composition? is this argument ($driver) really used?

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Nov 22, 2013

Contributor
Contributor

JJK801 commented Nov 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment