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
Drop oldstable dependency version support: bump *all* dependencies to current stable release #873
Conversation
… current stable release
0a84882
to
3fb7178
Compare
"jdorn/sql-formatter": "^1.2.16", | ||
"symfony/doctrine-bridge": "~2.7|~3.0|~4.0", | ||
"doctrine/doctrine-cache-bundle": "~1.2" | ||
"php": "^7.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since SF 4 requires PHP 7.1, this bundle should IMO do the same, at least until 5.0 GA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this needs to stay 7.1 for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to follow symfony's dependency versioning: the bundle can define its own dependency versions (and it should)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bundle for the Symfony eco-system and it is built for Symfony and should follow the Symfony release cycle they follow and document publicly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something we discussed, discussed and discussed again ad-nauseam.
If the package still has to live under doctrine/*
, then keep it under our maintenance with our constraints and our ability to maintain it.
If it is part of the Symfony LTS cycle yaddayaddawhatever, then let's push it under symfony/*
and call it a day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and the 7.1 users won't be able to upgrade Symfony.
This is not a problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Symfony eco-system still supports 7.1 and this bundle is built for that eco-system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again: not a problem. You get the new features when you get to upgrade.
As mentioned above, this is a bundle, and if it should be maintained outside the doctrine organisation then I don't understand why it is in here in first place. If the release goes hand in hand with symfony, make it part of the symfony subtree split..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because that is just where it ended up several years ago. Where it is in github is irrelevant imo. Github has features to handle this, we have outside collaborators who have the ability to maintain this bundle (and they do). You can control notifications per repository, etc.
I can't think of any reason why it can't be moved but I also don't think it is a very big deal for the repository to live here and follow the documented release lifecycle of Symfony.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don't think it is a very big deal for the repository to live here and follow the documented release lifecycle of Symfony.
It is a big deal for my inbox, from what I can see.
Alright then, closing as won't fix
here, will simply mute the repo.
- stage: Code Quality | ||
env: CODING_STANDARDS | ||
php: 7.1 | ||
php: 7.2 | ||
install: | ||
- travis_retry composer require -n --prefer-dist --dev doctrine/coding-standard:^5.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can now move this to composer.json directly. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 but would need the usual composer.lock
ballet too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it to separate PR, we can enable some 7.1+ sniffs that were excluded and also use use function
. 😉 cc @carusogabriel if you're interested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropping 7.1 now is a big no-go in the bundle. We should keep 7.1 support for now - it doesn’t hurt us and pushing dependencies just for the sake of it makes no sense at this time. Let’s drop 5.x and 7.0 to align the constraints with Symfony 4 - that will keep all users happy. We should also release a bug fix version that is available for users of 3.4 as that is still widely used.
That isn't |
Closing as "won't fix" due to discussions above. Dependency constraints will stay as they are. |
Ref: #869 (comment)