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

Merged
merged 6 commits into from Dec 16, 2013

Conversation

Projects
None yet
3 participants
@JJK801
Contributor

JJK801 commented Nov 19, 2013

Hi,

This sample of code implements possibility of fetch repositories packages.json using s3:// scheme.

Need to wait composer/composer#2434 to be merged.

Thanks,

Jérémy

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Nov 22, 2013

Contributor

composer's PR has been merged, if you're ok, you can merge this one

Contributor

JJK801 commented Nov 22, 2013

composer's PR has been merged, if you're ok, you can merge this one

@omansour

This comment has been minimized.

Show comment
Hide comment

👍

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Dec 8, 2013

Contributor

ping @naderman

Please, we really need this feature :)

Thanks

Contributor

JJK801 commented Dec 8, 2013

ping @naderman

Please, we really need this feature :)

Thanks

@naderman

This comment has been minimized.

Show comment
Hide comment
@naderman

naderman Dec 16, 2013

Owner

So since the composer PR was reverted this won't work. Are you working on a new composer PR?

Owner

naderman commented Dec 16, 2013

So since the composer PR was reverted this won't work. Are you working on a new composer PR?

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Dec 16, 2013

Contributor

It was remerged by Seldaek just after with his own fix on VcsRepository.

Contributor

JJK801 commented Dec 16, 2013

It was remerged by Seldaek just after with his own fix on VcsRepository.

@naderman

This comment has been minimized.

Show comment
Hide comment
@naderman

naderman Dec 16, 2013

Owner

Oh I saw your fix being closed and didn't have a chance to ask him yet :D

Owner

naderman commented Dec 16, 2013

Oh I saw your fix being closed and didn't have a chance to ask him yet :D

@naderman

This comment has been minimized.

Show comment
Hide comment
@naderman

naderman Dec 16, 2013

Owner

Alright this generally looks good then, but I'd prefer if you could merge the getContents and download methods now since the code is mostly duplicated. Could you do that? Cheers!

Owner

naderman commented Dec 16, 2013

Alright this generally looks good then, but I'd prefer if you could merge the getContents and download methods now since the code is mostly duplicated. Could you do that? Cheers!

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Dec 16, 2013

Contributor

no problem

Contributor

JJK801 commented Dec 16, 2013

no problem

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Dec 16, 2013

Contributor

Is it what you expected @naderman ?

Contributor

JJK801 commented Dec 16, 2013

Is it what you expected @naderman ?

@naderman

This comment has been minimized.

Show comment
Hide comment
@naderman

naderman Dec 16, 2013

Owner

Yes, but there is a problem now. It still has to use the guzzle mechanism with fopen for writing because that streams the content. But the way you wrote it with file_put_contents now it loads the entire file into RAM which can be an issue with big downloads.

Owner

naderman commented Dec 16, 2013

Yes, but there is a problem now. It still has to use the guzzle mechanism with fopen for writing because that streams the content. But the way you wrote it with file_put_contents now it loads the entire file into RAM which can be an issue with big downloads.

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Dec 16, 2013

Contributor

You're right, must be much better now

Contributor

JJK801 commented Dec 16, 2013

You're right, must be much better now

@naderman

This comment has been minimized.

Show comment
Hide comment
@naderman

naderman Dec 16, 2013

Owner

Ok cool, this looks good. Thanks for the patience and good work :)

Owner

naderman commented Dec 16, 2013

Ok cool, this looks good. Thanks for the patience and good work :)

naderman added a commit that referenced this pull request Dec 16, 2013

Merge pull request #5 from JJK801/enalble-s3-for-repositories
Add repositories (packages.json) management using S3

@naderman naderman merged commit 77373cd into naderman:master Dec 16, 2013

@naderman

This comment has been minimized.

Show comment
Hide comment
@naderman

naderman Dec 16, 2013

Owner

Also tagged a new version 0.2.4

Owner

naderman commented Dec 16, 2013

Also tagged a new version 0.2.4

@JJK801

This comment has been minimized.

Show comment
Hide comment
@JJK801

JJK801 Dec 16, 2013

Contributor

Thanks!

Contributor

JJK801 commented Dec 16, 2013

Thanks!

@JJK801 JJK801 deleted the JJK801:enalble-s3-for-repositories branch Dec 16, 2013

@omansour

This comment has been minimized.

Show comment
Hide comment
@omansour

omansour Dec 16, 2013

Excellent :)

Thanks to you both

Le 16 déc. 2013 à 17:25, Nils Adermann notifications@github.com a écrit :

Also tagged a new version 0.2.4


Reply to this email directly or view it on GitHub.

Excellent :)

Thanks to you both

Le 16 déc. 2013 à 17:25, Nils Adermann notifications@github.com a écrit :

Also tagged a new version 0.2.4


Reply to this email directly or view it on GitHub.

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