search

sampart / WhiteOctoberTCPDFBundle

A bundle to facilitate using TCPDF for PDF generation in Symfony applications
89 stars 43 forks source link

Add version constraint for tecnickcom/tcpdf #53

Closed bertoost closed 5 years ago

bertoost commented 5 years ago

It is very bad practice to just include any version from another package. This will cause updates when running composer update while your package isn't update but the required dependency is.

sampart commented 5 years ago

Hi @bertoost, thanks for getting involved.

You’re right - the asterisk version constraint is a bad idea! However, I think this is a rare situation where we should leave it in.

The alternative would be to constrain to the latest major version of TCPDF and then create new releases of the bundle (with updated constraint) whenever new major versions of TCPDF are released, once we’ve confirmed that the new TCPDF version works with our bundle.

However, we unfortunately don’t have time to monitor TCPDF for releases and update the project accordingly, so this would introduce delays for users in using new versions. More importantly, however, we definitely don’t have time for regression testing when new TCPDF versions are released (and unfortunately don’t have automated tests to do this for us - it would be good if we did, of course!) Given that, we’d end up with a situation where TCPDFBundle was restricted to v6 of TCPDF indefinitely (unless other people could confirm that it worked on newer versions for us).

I think that, although it does introduce risk as you mention, allowing any version is better than this potential impasse.

(This is particularly true as this bundle is a very thin wrapper which just makes a configurable TCPDF object available via the container, and so it’s rarely affected by BC-breaking changes in TCPDF anyway.)

It’s worth noting that end-users can still control the version of TCPDF used by adding it to their project’s own composer.json file. I will add something to the README about this.

The asterisk constraint shows to users that we’re not tracking TCPDF versions effectively, which is not ideal but is actually a true reflection of the project! Adding a proper constraint would imply a level of release conformance that we couldn’t live up to.

I appreciate that this isn’t an ideal answer and I’m sorry we don’t have the time for a more robust process here. Thanks again for your involvement with the project.

sampart commented 5 years ago

The documentation PR mention above is here. I'd be grateful for your thoughts, @bertoost.

bertoost commented 5 years ago

Good to note in the README right below the composer require and I do understand it is more time consuming for you to make version constraints. But my TCPDF was updated while I didn't update your bundle. That was kinda frustrating since stuff didn't work. From a minor or patch version that isn't so bad, but for a major change this is very bad.

You also can decide to constraint on a higher level, like "~6.2" and then only add updates with bigger releases.

Therefor good to know I can require it myself. Didn't know that works prior to your requirement.

sampart commented 5 years ago

Thanks for the information. I'm sorry this broke for you. If you get a chance, please do raise issues for anything that doesn't work with later TCPDF versions and we'll see what we can do.