staudenmeir / belongs-to-through

Laravel Eloquent BelongsToThrough relationships
MIT License
1.15k stars 88 forks source link

Backward compatibility break in minor version #76

Closed denniscoorn-paqt closed 1 year ago

denniscoorn-paqt commented 1 year ago

First of all, great package, works like a charm, thanks! 👌

I noticed that upgrading the package to the latest minor v2 release was not possible due to a change in the composer version constraints. The release notes of v2.13 states that support for Laravel 10 has been added, but at the same time support for Laravel 9 has been dropped. The later is not mentioned in the release notes and is not something I would have expected.

I think it's fair to say that it is generally assumed that minor versions can introduce new features or support for new versions of dependent packages, but will not introduce backward compatibility breaks. With major versions, however, it can be expected that it possibly can introduce backward compatibility breaks or drop support for certain dependent versions.

My suggestion would be to create a v2.13.1 release with the following change in the composer.json:

"php": "^8.0 | ^8.1",
"illuminate/database": "^9.0 | ^10.0"

This will unambiguously reflect that this package supports PHP 8.0 and 8.1, and Laravel 9 and 10. Based on the current codebase I don't see why there would be no support for both PHP 8.0 and 8.1, and also Laravel 9 and Laravel 10. Even the fact that Laravel 10 is only supported for PHP 8.1 shouldn't be a problem, because Composer is able to figure out, for instance, that a combination of PHP 8.0 and Laravel 9 falls within the allowed version constraints.

Whenever you want to only support PHP 8.1 and Laravel 10 I would suggest to create a v3.0.0 release where you explicitly drop support for PHP 8.0 and Laravel 9.

Keep up the good work! ✌️

staudenmeir commented 1 year ago

Hi @denniscoorn-paqt, My packages always have a dedicated (minor) version for each Laravel version. So a new version doesn't mean that I drop support for older Laravel versions – that's how I add support for new Laravel versions. I backport new features and bug fixes to older package versions.

I noticed that upgrading the package to the latest minor v2 release was not possible due to a change in the composer version constraints.

What did you do here? Did you explicitly request v2.13? composer update [staudenmeir/belongs-to-through] should never try to install a version that doesn't work with your other dependencies (i.e. the Laravel version).

denniscoorn-paqt commented 1 year ago

I backport new features and bug fixes to older package versions.

Very nice, this answers the underlying question I had with regards to the versioning.

And to clarify, it was not my intention to say that your package adds or drop certain support as a whole, but as I'm more used to using semantic versioning, I wouldn't have expected such a change in version constraints within a minor release. In the end, this also works, especially when older version are also being updated. 👍

What did you do here? Did you explicitly request v2.13?

Indeed, that is what happend. After a composer require staudenmeir/belongs-to-through it was added to the composer.json as "staudenmeir/belongs-to-through": "*". However, I prefer to be a bit more precise with the version constraints so I checked which version had been installed, that was v2.12, and I saw that a newer version was released, v2.13. Based on the semantic versioning approach I assumed that such a minor release wouldn't be a problem to install, so I changed the version constraint to "staudenmeir/belongs-to-through": "^2.13" and that wasn't working. I could have used "staudenmeir/belongs-to-through": "^2.0" which would have worked fine, but it's kind of habit to use the latest available release when adding new packages.

Anyway, a lot of words to say: thanks for the quick reply and it was just a "user error" based on assumptions 😂