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

Added sort-packages into config #4716

Merged
merged 2 commits into from Dec 22, 2015

Conversation

Projects
None yet
3 participants
@hanovruslan
Copy link
Contributor

hanovruslan commented Dec 18, 2015

#4715 Added sort-packages into config

@hanovruslan hanovruslan changed the title #4715 Added sort-packages into config Added sort-packages into config Dec 19, 2015

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Dec 21, 2015

Please also add a way to programmatically set this new config flag while you are at it.

I forgot that adding it to the config command should take care of that already.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Dec 21, 2015

  • Needs a test: just add something to the require scenarios, make sure it runs with the config set to true, verify the result is sorted.
  • Needs documentation.
@hanovruslan

This comment has been minimized.

Copy link
Contributor

hanovruslan commented Dec 21, 2015

@alcohol

Needs a test: just add something to the require scenarios

please tell in which (test?) file ?

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Dec 21, 2015

I had a look at tests/Composer/Test/Fixtures/installer, but it actually does not seem possible to write a test for this particular scenario with the current tools available I'm afraid. Was hoping that we would be able to check that given a composer.json and running require x, we could check the resulting composer.json; but alas no such use-case is yet available among the installer tests.

@alcohol alcohol removed the Needs Test label Dec 21, 2015

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Dec 21, 2015

Just some documentation will be sufficient. Check the CLI and config document.

@hanovruslan

This comment has been minimized.

Copy link
Contributor

hanovruslan commented Dec 21, 2015

👍

@hanovruslan

This comment has been minimized.

Copy link
Contributor

hanovruslan commented Dec 22, 2015

@alcohol this would be enough ?

@Seldaek

This comment has been minimized.

Copy link
Member

Seldaek commented Dec 22, 2015

Thanks

@Seldaek Seldaek merged commit 3861518 into composer:master Dec 22, 2015

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Seldaek added a commit that referenced this pull request Dec 22, 2015

@hanovruslan hanovruslan deleted the hanovruslan:add-sort-packages-into-config branch Jan 7, 2016

aminztw pushed a commit to aminztw/composer that referenced this pull request Jan 1, 2017

@localheinz localheinz referenced this pull request Dec 28, 2017

Merged

Fix: Add 'sort-packages' to composer-schema.json #6947

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