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

[Composer] Allow imagine-library version 0.7.0 #958

Merged
merged 1 commit into from
Jul 9, 2017

Conversation

robfrawley
Copy link
Collaborator

Q A
Branch? 1.0
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #935
License MIT
Doc PR

This pull request adds the ability to utalize version 0.7.0 of the Imagine Library, and raises our upper limit constraint to 0.8.0. By default, this will actually cause most people to upgrade from 0.6.0 to 0.7.0 of Imagine Library, but it allows those who explicitly constrain their project to Imagine Library 0.6.0 to remain on that version, as well. It provides a cleaner, alternate approach to #935 with passing tests. Fixes an important issue introduced by 3.4.3 of the imagick extension; see php-imagine/Imagine#547.

@robfrawley robfrawley added Level: Bug 🐞 This item involves a legitimate regression (bug) to existing functionality. Level: New Feature 🆕 This item involves the introduction of new functionality. State: Reviewing This item is being reviewed to determine if it should be accepted. labels Jul 3, 2017
@robfrawley robfrawley added this to the 1.8.1 milestone Jul 3, 2017
@robfrawley robfrawley self-assigned this Jul 3, 2017
@@ -19,7 +19,7 @@
},
"require": {
"php": "^5.3.9|^7.0",
"imagine/Imagine": "^0.6.3,<0.7",
"imagine/Imagine": "^0.6.3|^0.7.0,<0.8",
Copy link

Choose a reason for hiding this comment

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

The ,<0.8 is not necessary. ^0.7.0 means >=0.7.0 <0.8.0.

Copy link
Collaborator Author

@robfrawley robfrawley Jul 3, 2017

Choose a reason for hiding this comment

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

I realize; I believe the current syntax was used to provide clarity, as even before the change when it was ^0.6.3,<0.7, the <0.7 was unnecessary. It's important to note that the rule you mentioned only applies for fully enumerated versions [above 1.x], so it may be a good idea to retain the upper constraint so when someone comes along and changes it to, say ^1.0, we don't open the floodgates.

There are a bunch of ways to write this, such as ^0.6.3|^0.7.0,<0.8, >=0.6.3,~0.6,<0.8, >=0.6.3,<0.8 or ~0.6.3|~0.7.0 (the latter of which I personally prefer), but the original intention seems to have been verbosity, which the current syntax provides, no?

If anything, I'd prefer ~0.6.3|~0.7.0, honestly.

Copy link

Choose a reason for hiding this comment

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

^0.7 and ^0.7.0 are actually equivalent. But I understand your point. It is more clear for those who don't know how the the caret operator works.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's important to note that the rule you mentioned only applies for fully enumerated versions

I meant to append that with "above 1.x" and I obviously used the wrong example of 0.7 where I intended to use 1.0. Was typing quickly. ;-)

@robfrawley robfrawley changed the title Support for latest Imagine library (version 0.7.0) Enable imagine-library version 0.7.0 support Jul 8, 2017
@robfrawley robfrawley changed the title Enable imagine-library version 0.7.0 support [Composer] Enable imagine-library version 0.7.0 support Jul 8, 2017
@robfrawley robfrawley changed the title [Composer] Enable imagine-library version 0.7.0 support [Composer] Allow imagine-library version 0.7.0 Jul 8, 2017
@alexwilson
Copy link
Collaborator

I guess to avoid any complaints on bumping most from .6 to .7, we potentially release under a minor version?

Either way 👍

@alexwilson alexwilson merged commit c13bb62 into liip:1.0 Jul 9, 2017
@trsteel88
Copy link
Contributor

@robfrawley can you tag a version for this please?

@robfrawley
Copy link
Collaborator Author

The next release (which will be tagged as 1.9.0) is scheduled for this coming Monday (2017-07-17). The following PRs need to be properly reviewed and merged for that release: #933, #937, and #941.

You can reference the 1.9.0 milestone to track progress. With any luck, that date won't slip; with some luck, that date may be moved up. We'll see. :-)

@trsteel88
Copy link
Contributor

Any chance we can get this tagged sooner as a patch version @robfrawley? It's my understanding that bug fixes can/should be tagged straight away? e.g 1.8.1

Just to be a pain, we've just got this issue occurring on a number of sites where we have to manually edit the file on the server :(

@robfrawley
Copy link
Collaborator Author

robfrawley commented Aug 31, 2017

@trsteel88 See #983. As for quick releases (even for bugs and other issues), the lax schedule is the result of the minimum time available to only three "sporadically active" maintainers. We try our best. :-)

@robfrawley robfrawley mentioned this pull request Aug 31, 2017
@trsteel88
Copy link
Contributor

Thank you. Appreciate the quick response.

@cedricziel
Copy link
Collaborator

@trsteel88 As for fixes, you can use your own bugfixes by adding your fork as composer repository (https://getcomposer.org/doc/05-repositories.md#vcs) and require version dev-$yourbranchname as 1.8.1

@robfrawley
Copy link
Collaborator Author

@trsteel88 The 1.9.0 release is live! Enjoy.

@trsteel88
Copy link
Contributor

Thanks @robfrawley

Good to know @cedricziel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Level: Bug 🐞 This item involves a legitimate regression (bug) to existing functionality. Level: New Feature 🆕 This item involves the introduction of new functionality. State: Reviewing This item is being reviewed to determine if it should be accepted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants