Skip to content

Commit

Permalink
excluding some unecessary file from the composer downloaded tarball
Browse files Browse the repository at this point in the history
  • Loading branch information
mikeSimonson committed Jul 29, 2015
1 parent aa34a11 commit 3a787de
Showing 1 changed file with 14 additions and 0 deletions.
14 changes: 14 additions & 0 deletions .gitattributes
@@ -0,0 +1,14 @@
build/* export-ignore
tests/* export-ignore
composer.lock export-ignore
.coveralls export-ignore
.gitattributes export-ignore
.gitignore export-ignore
.scrutinizer.yml export-ignore
.travis.yml export-ignore
box.json export-ignore
build.properties.dev export-ignore
build.xml export-ignore
composer.json export-ignore
composer.lock export-ignore
LICENSE export-ignore

This comment has been minimized.

Copy link
@stof

stof Sep 15, 2015

Member

the license must not be excluded. It would violate the license if you don't ship it

10 comments on commit 3a787de

@kdambekalns
Copy link

Choose a reason for hiding this comment

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

Why on earth would you exclude the composer.json? It contains vital information, and assuming it is only ever fetched via packagist and or used internally in composer itself is wrong… Flow e.g. uses it to look at dependencies on it's own, and I'd always assume a composer package I download–whichever way–contains a manifest.

@stof
Copy link
Member

@stof stof commented on 3a787de Oct 1, 2015

Choose a reason for hiding this comment

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

@kdambekalns tools running locally should rely on the metadata about installed packages (or on the lock file) IMO, as these are the metadata used by composer (if there is a mismatch, which can happen if you use a package repository while the code has a composer.json, the repository wins, not the file in the downloaded code)

@mikeSimonson
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well despite not being absolutely necessary, I am not against putting it back in the tarball.
In this case, it's an app not a library, so we should maybe put both the composer.json and the composer.lock in the archive ? @stof @kdambekalns

@kitsunet
Copy link

Choose a reason for hiding this comment

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

Would be great to have both in :)

@kdambekalns
Copy link

Choose a reason for hiding this comment

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

Well, for me it is definitely a library, but YMMV. Anyway, does no harm at all, so it would be nice to have both in.

@aertmann
Copy link

Choose a reason for hiding this comment

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

Since composer.json has become the place to store metadata about a repository in general, removing it makes that information unavailable to everyone downloading the archive. E.g. it's the only place mentioning the project website, authors, minimum php version. That in itself should be enough reason to keep it IMO.

@aertmann
Copy link

Choose a reason for hiding this comment

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

@stof @mikeSimonson: hey, any update on this?

@mikeSimonson
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aertmann Thanks for the ping.
I will put back the composer.json as it can be usefull if you download the tarball by another means than composer.
As for the composer.lock it should also be remove from the gitattributes but it won't put it in the archive as it isn't tracked by git.

@mikeSimonson
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aertmann fixed in #406

@aertmann
Copy link

Choose a reason for hiding this comment

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

@mikeSimonson thanks! much appreciated

Please sign in to comment.