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

sensio/generator-bundle update #156

Merged
merged 3 commits into from
Jan 24, 2017
Merged

sensio/generator-bundle update #156

merged 3 commits into from
Jan 24, 2017

Conversation

xrow
Copy link
Contributor

@xrow xrow commented Jan 22, 2017

Allow newer versions of sensio/generator-bundle.

Allow newer versions of sensio/generator-bundle.
@xrow xrow closed this Jan 22, 2017
@xrow xrow deleted the patch-2 branch January 22, 2017 01:51
@xrow xrow restored the patch-2 branch January 22, 2017 01:52
@xrow xrow reopened this Jan 22, 2017
@@ -24,7 +24,7 @@
"symfony/swiftmailer-bundle": "~2.3",
"symfony/monolog-bundle": "~2.4",
"sensio/distribution-bundle": "^3.0.36|^4.0.6|^5.0",
"sensio/generator-bundle": "~2.3",
"sensio/generator-bundle": "*",
Copy link
Contributor

@andrerom andrerom Jan 23, 2017

Choose a reason for hiding this comment

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

We would typically rather bump it to "~3.0", to avoid sudden removal of features we use in the platform.

@bdunogier Do you remember what we use in the bundle so we can check it is still there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is used directly by eZ Platform, but rather it's a part of Symfony Standard Edition. In any case, I'd rather go for ^2.3|^3.0 than unbound constraint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked you don`t use it at all, but of course you want to to start a new bundle.

Copy link
Contributor Author

@xrow xrow Jan 23, 2017

Choose a reason for hiding this comment

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

One last comment. @emodric I wouldn't define a constraint, if there isn't a real one.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xrow What does that mean? It's a bad practice to use unbound constraints, as stuff can break at any moment with them.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xrow I'm not sure I understand you. eZ kernel does not have a dependency on sensio/generator-bundle, therefore, you can change it yourself in composer.json in your own projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I have my requirement on ezplatform and not on ezpublish-kernel since I want to include all of eZ.

Copy link
Contributor

Choose a reason for hiding this comment

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

Uhm... But that's not how it works :)

You should not require ezystems/ezplatform in your project, your project should be a fork of ezsystems/ezplatform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maintaining a fork of ezsystems/ezplatform is to complicated for us. Having the dependency is way easier. Please lets concentrate on solving this now.

Copy link
Member

Choose a reason for hiding this comment

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

No problem with the dependency update, but please let's use ^2.3|^3.0. I'd rather avoid the warning on composer validate :)

@xrow
Copy link
Contributor Author

xrow commented Jan 23, 2017

Can one rerun CI it looks like it is not my fault. Don`t know how to trigger that.

@andrerom
Copy link
Contributor

Can one rerun CI it looks like it is not my fault. Don`t know how to trigger that.

It will be rerun once PR is amended or new commit is added to fix the inline comment ;)

@andrerom andrerom merged commit 77c022a into ezsystems:master Jan 24, 2017
@xrow xrow deleted the patch-2 branch January 24, 2017 12:24
@andrerom
Copy link
Contributor

andrerom commented Jan 24, 2017

Merged, thanks @xrow and @emodric :)

andrerom pushed a commit that referenced this pull request Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants