Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

removed composer download #1155

Merged
merged 1 commit into from Nov 11, 2012

Conversation

Projects
None yet
5 participants
Contributor

bamarni commented Sep 28, 2012

No description provided.

Contributor

GromNaN commented Sep 28, 2012

This was testing that composer can be downloaded.

Contributor

bamarni commented Sep 28, 2012

makes sense :)

@bamarni bamarni closed this Sep 28, 2012

Contributor

pborreli commented Sep 28, 2012

sorry but it doesn't make sense to me, travisci installed composer on their box to avoid the huge trafic generated by that kind of downloads.
I don't see how it is necessary to have the top recent composer to install deps needed to run PHPUnit
I don't see how travisci can be considered as a monitoring system for the download part of getcomposer.org

Contributor

bamarni commented Sep 28, 2012

imo he's right the download should be tested as it's how people get composer.

But indeed it is not appropriate here, maybe this should be tested in getcomposer.org repo.

@bamarni bamarni reopened this Sep 28, 2012

Contributor

pborreli commented Sep 28, 2012

well I'm pretty sure getcomposer.org is monitored with some pingdom like service and even if it fails it won't be caused by a composer/composer commit

Contributor

stof commented Sep 28, 2012

@pborreli The phar is build automatically by a hook on the composer/composer repo. If a commit in composer breaks the compilation of the phar, the testsuite will fail to download the phar.
It cannot be tested with travis on the getcomposer.org repo as there is no commit there in such case.

So for me, it makes sense to download the composer phar here (because it is the composer testsuite; other packages should indeed prefer the available phar)

Contributor

pborreli commented Sep 28, 2012

@stof I guess if the compilation of the phar fails the older version of the phar file will still be accessible, if not it sounds stupid

Contributor

stof commented Sep 28, 2012

@pborreli If compiling the phar breaks, the old one would be kept. But this does not happen often. In most cases where a commit broke the compilation, the compilation process was working but producing a broken phar.

A case where this happened was during the refactoring of the autoloading, introducing a new file. The compiler was not updated so the phar was broken.
In such case, the composer.phar would simply be broken on getcomposer.org (the site also keep a phar for each tagged release of composer which would still be available).

Contributor

pborreli commented Sep 28, 2012

@stof so maybe it would be appropriate to test the phar building inside the travisci job, it would show errors at PR time instead of master.

Contributor

stof commented Sep 28, 2012

@pborreli what would be needed is to compile the phar and run the testsuite with it in fact.

And then, the phar on getcomposer.org should ideally be rebuild only when the testsuite is successful but this is a bit tricky if it needs to wait for the updated Travis status (I'm not even sure this is possible properly as we would need the status for a commit, not for a branch).

Contributor

pborreli commented Sep 28, 2012

@stof and maybe instead of using github hook we should use travisci webhook

notifications:
  webhooks:
    urls:
      - http://your-domain.com/notifications
    on_success: always
    on_failure: never

It sends this payload where you can filter only notification coming from original master and not PRs.

@Seldaek Seldaek merged commit 51eb026 into composer:master Nov 11, 2012

Owner

Seldaek commented Nov 11, 2012

Merged this and added a test (see above) that builds the phar

digitalkaoz pushed a commit to digitalkaoz/composer that referenced this pull request Nov 22, 2013

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