symfony / orm-pack

A Symfony Pack for Doctrine ORM
MIT License
1.82k stars 18 forks source link

Bump DoctrineMigrationsBundle version #13

Closed IonBazan closed 5 years ago

IonBazan commented 5 years ago

DoctrineMigrationsBundle v2.0.0 has been released: https://github.com/doctrine/DoctrineMigrationsBundle/tree/v2.0.0

ubermuda commented 5 years ago

Hey so I know I'm late to the party but this change is breaking BC (main namespace has been renamed from what I can see) so maybe it should be tagged with a different major version?

angelk commented 5 years ago

@ubermuda I don't see any changes in the namespace, could you elaborate?

colinodell commented 5 years ago

I think @ubermuda is referring to the BC breaks in doctrine/migrations 2.0 which is part of the dependency tree:

doctrine/migrations v2.0.0
└──doctrine/doctrine-migrations-bundle v2.0.0 (requires doctrine/migrations ^2.0)
   └──symfony/orm-pack v1.0.6 (requires doctrine/doctrine-migrations-bundle ^1.3|^2.0)

symfony/orm-pack has wide constraints in order to be compatible with a wide range of packages and versions:

Package Lowest Version Highest Version
doctrine/annotations v1.0 v1.6.0
doctrine/cache v1.4.2 v1.8.0
doctrine/collections v1.2 v1.5.0
doctrine/common v2.5.0 v2.10.0
doctrine/dbal v2.5.0 v2.9.2
doctrine/doctrine-bundle 1.6.10 1.10.1
doctrine/doctrine-cache-bundle 1.2.0 1.3.5
doctrine/doctrine-migrations-bundle v1.3.0 v2.0.0
doctrine/event-manager n/a v1.0.0
doctrine/inflector v1.0 v1.3.0
doctrine/instantiator 1.0.4 1.1.0
doctrine/lexer v1.0 v1.0.1
doctrine/migrations v1.1.0 v2.0.0
doctrine/orm v2.5.11 v2.6.3
doctrine/persistence n/a v1.1.0
doctrine/reflection n/a v1.0.0
jdorn/sql-formatter v1.1.0 v1.2.17
psr/log 1.0.0 1.1.0
symfony/asset v2.7.0 n/a
symfony/cache n/a v4.2.2
symfony/config v2.7.0 v4.2.2
symfony/console v2.7.0 v4.2.2
symfony/contracts n/a v1.0.2
symfony/debug v2.6.2 v4.2.2
symfony/dependency-injection v2.7.0 v4.2.2
symfony/doctrine-bridge v2.7.0 v4.2.2
symfony/event-dispatcher v2.5.9 v4.2.2
symfony/filesystem v2.3.0 v4.2.2
symfony/finder n/a v4.2.2
symfony/framework-bundle v2.7.0 v4.2.2
symfony/http-foundation v2.5.4 v4.2.2
symfony/http-kernel v2.7.0 v4.2.2

And several others.

(This was generated by cloning v1.0.6 of this repo and running composer update vs composer update --prefer-lowest).

This allows almost anyone with a relatively modern version of Symfony to automatically pull the latest version Doctrine into their projects by simply requiring symfony/orm-pack! Composer will figure out whatever the latest version is and give you that; you'll continue to get the latest versions each time you composer update too unless you provide additional constraint around what your code does and does not support (or you use composer.lock and only update specific packages as needed).

So ultimately I don't believe this is a bug or a BC break as this package is not intended to lock you into a specific major version - that's something you'll need to manage yourself.

ubermuda commented 5 years ago

Sorry I should have given more details, I am indeed refering to the change @colinodell is talking about.

Anyway, what you are saying here is that this pack can't guarantee that a composer update symfony/orm-pack will not break your app and that it is "by design", right?

colinodell commented 5 years ago

That is correct. This package basically tells Composer "give me some version of Doctrine and whatever it needs, preferably the newest one". If your code relies on a specific version, you'll want to be sure to specify that as a constraint in your own composer.json file.

Here's another way to think about it - pretend you composer require some package named foo/bar that depends on also the Guzzle HTTP client (a popular project with many major versions). Composer will automatically select a version of Guzzle for you. Even though you didn't directly require that specific package yourself, there's nothing stopping you from using Guzzle in your own code too. However, if foo/bar suddenly changes to a new major version of Guzzle (which they can technically do in a non-major version in some cases) Composer will gladly install that and break your app because you never told Composer "hey I actually need a certain version of Guzzle myself" - you actually said: I'll use whatever foo/bar happens to use, I don't care what that is.

I know it's not the best user experience, but I hope I helped to explain why it works this way.

ubermuda commented 5 years ago

Thanks for taking the time to explain @colinodell. I understand the reasoning and accept it as is, but do not agree with it :) IMHO there is a huge difference between "I use doctrine because it is in the pack that I installed specifically for using doctrine" and "I use guzzle because it happens to be a dependency of a package that I installed with no intention whatsoever at the time to install guzzle". The former is a clear intent of use while the second is just a form of programation by side-effect ("oh look, this class exists in my project, I'm not sure why but I don't really care, let's use it"), which you will agree is to be frowned upon.

I respect your reasoning and will not really try to convince you of my views (just wanted to clarify my point of view, and I already unpacked all my packs so I should be fine eitherway), but I really feel like Symfony Packs should either 1. follow semver OR 2. explicitely warn the user that a composer update symfony/orm-pack (or any other pack) MIGHT break things because it is NOT following semver.

Anyway, thanks again for your time and for the great work every one of you are puting into this!

gharlan commented 5 years ago

IMHO there is a huge difference between "I use doctrine because it is in the pack that I installed specifically for using doctrine" and "I use guzzle because it happens to be a dependency of a package that I installed with no intention whatsoever at the time to install guzzle".

👍

colinodell commented 5 years ago

Thanks for taking the time to explain @colinodell. I understand the reasoning and accept it as is, but do not agree with it :) IMHO there is a huge difference between "I use doctrine because it is in the pack that I installed specifically for using doctrine" and "I use guzzle because it happens to be a dependency of a package that I installed with no intention whatsoever at the time to install guzzle".

I completely agree! It's unfortunate that Composer can't make that distinction. Perhaps this could be solved with some kind of Composer plugin that, when told to include a pack, instead adds the pack's dependencies directly to your composer.json with sensible constraints, instead of requiring the pack itself? Just a random thought I had.

explicitely warn the user that a composer update symfony/orm-pack (or any other pack) MIGHT break things because it is NOT following semver.

I agree on this too - the behavior is not what most people expect, so some kind of warning somewhere isn't a bad idea.

ubermuda commented 5 years ago

Perhaps this could be solved with some kind of Composer plugin that, when told to include a pack, instead adds the pack's dependencies directly to your composer.json with sensible constraints, instead of requiring the pack itself? Just a random thought I had.

We are actually discussing this right now with my team and according to http://fabien.potencier.org/symfony4-unpack-the-packs.html there is an --unpack option to the Flex that does just that. We just think it should be by default :)

Another thing, I have looked for an "official" description of what packs are and how they work but couldn't find one, did I miss something or could it be something lacking at the moment?