Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit the composer.lock file #196

Merged
merged 1 commit into from
Dec 3, 2016
Merged

Commit the composer.lock file #196

merged 1 commit into from
Dec 3, 2016

Conversation

ezzatron
Copy link
Contributor

This PR does a few related things:

  • Adds the composer.lock file. This is recommended by Composer, and should improve build times, and help when switching branches with different dependencies.
  • Cleans up the .gitignore file, removing the composer.lock entry, and a bunch of entries that should really be in a global .gitignore file, such as .DS_Store and .idea.
  • Uses slash prefixes in .gitignore where appropriate.
  • Updates composer.json to use the platform config option, so that dependencies get locked at appropriate versions for our minimum PHP constraint.
  • Updated dependencies to mitigate some Composer warnings about abandoned packages.

In regards to committing the lock file, there is the small downside that parallel changes to composer.lock always cause merge conflicts, but this is simple to fix, and the benefits outweigh the problems.

@ezzatron ezzatron changed the title Updated dependencies, and commit the composer.lock file. Commit the composer.lock file Nov 29, 2016
@codecov-io
Copy link

codecov-io commented Nov 29, 2016

Current coverage is 96.74% (diff: 100%)

Merging #196 into master will not change coverage

@@             master       #196   diff @@
==========================================
  Files            20         20          
  Lines           645        645          
  Methods         162        162          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            624        624          
  Misses           21         21          
  Partials          0          0          

Powered by Codecov. Last update d04dfa4...f529ac4

@brianium
Copy link
Member

@ezzatron - sounds good to me. I have rarely checked in lock files for these sort of projects, but I can't really think of a good reason why. Peridot is an application so 👍

I am super unfamiliar with the platform config - will this create issues with people using Peridot in a PHP 7 environment with newer deps? Our constraint is >= 5.4 so probably not right?

As for the .gitignore - I am not opposed to this. This would not be the first time I was called out on this. I just don't understand the need for it. Just good form? Keeps the .gitignore from bloating?

@ezzatron
Copy link
Contributor Author

ezzatron commented Nov 29, 2016

Re platform; it will change the dependencies that people who download a phar, or install globally, would get, because Composer will only choose dependencies that are compatible with PHP 5.4. I see this as a good thing though. You wouldn't want to globally install Peridot under PHP 7, then switch to PHP 5.4 and have it break suddenly.

For people who are bringing in Peridot under require-dev via Composer, the platform section is ignored, and they will get the latest deps possible. So it doesn't hold back those who are working exclusively with later versions of PHP.

There are some potential problems I suppose. The same kind of problems that arise with having ^2|^3 for our symfony/console constraint, in that we're only testing under CI with a sub-set of the dependencies that we support. The best way to fix that without going insane is probably to phase out support for older versions of PHP, and other dependencies, over time.

Re .gitinore, yeah, it's mostly to tackle bloat. It also means you don't have to duplicate this config across multiple repos, and it's easier to tell what transient files and directories are used by build processes and coverage reports without a bunch of cruft in there.

@brianium brianium merged commit 2f5cdb1 into master Dec 3, 2016
@brianium brianium deleted the commit-composer-lock branch December 3, 2016 15:06
@ezzatron
Copy link
Contributor Author

ezzatron commented Dec 4, 2016

Awesome! I'll rebase the other branches off of this one ASAP.

@ezzatron ezzatron modified the milestone: Next release Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants