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

Fix gaufrette version #1283

Closed
wants to merge 1 commit into from
Closed

Fix gaufrette version #1283

wants to merge 1 commit into from

Conversation

ramol
Copy link
Contributor

@ramol ramol commented Mar 26, 2014

This resolve conflict after KnpLabs/KnpGaufretteBundle@f330085 PR

@winzou
Copy link
Contributor

winzou commented Mar 26, 2014

Do we use gaufrette directly in Sylius? Otherwise we should have only dependency on the bundle.

@@ -41,7 +41,7 @@
"sylius/resource-bundle": "0.9.*@dev",
"friendsofsymfony/user-bundle": "2.0.*@dev",
"jms/serializer-bundle": "0.12.*",
"knplabs/gaufrette": "0.2.*@dev",
"knplabs/gaufrette": "~0.1.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be completly removed, if the bundle uses a stable dependency. The dev constraint was there, to resolve conflicts, as Sylius has a stable minimum-stability.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the main composer.json + lock file should be adjusted.

Copy link
Member

Choose a reason for hiding this comment

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

@ramol Could you update your patch, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about PR #1284 and remove all gaufrette dependency?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ramol The #1284 is totally different approach, and it is still WIP, so till it will be done, this one is correct (after adjusting it to mentioned comments).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh but main composer.json don't have dependency to "knplabs/gaufrette", I can PR lock after update.
When I try update SyliusCoreBundle I got also problem with doctrine/doctrine-bundle ~1.3@dev -> no matching package found. Stof suggests doctrine/DoctrineBundle#276 (comment)
but we have dependency to doctrine/doctrine-cache-bundle in SyliusSettingsBundle. It's like spider web :D Maybe somebody else can resolve this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After PR KnpLabs/KnpGaufretteBundle@ae598e6 we don't need to change anything with Gaufrette and we can close this PR

Standard Edition composer update works like a charm
:D

@stloyd stloyd added the Bug Fix label Mar 26, 2014
@stloyd stloyd added this to the 1.0.0-BETA1 milestone Mar 26, 2014
@mirague
Copy link

mirague commented Mar 27, 2014

Would be nice to see this through soon, can't run composer update due to this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants