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

Improve semver ranges in bower.json #1671

Closed
wants to merge 1 commit into from

Conversation

scotje
Copy link

@scotje scotje commented Jul 18, 2014

Some of the previous ranges didn't really make sense and seemed unintentionally restrictive. (e.g. Allowing jQuery 2.1.0 but no other 2.1.x releases.)

If there was a method behind the previous ranges, feel free to disregard. :)

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a8a5d35 on scotje:bower_semver_ranges into be4cd62 on marionettejs:master.

@scotje
Copy link
Author

scotje commented Jul 19, 2014

I'll close this and leave you guys in peace (or whatever the equivalent of peace is in a flat dep graph world).

@scotje scotje closed this Jul 19, 2014
@jamesplease
Copy link
Member

@scotje! Thanks for raising this issue! I'm going to reopen this because I'd like for us to take this into consideration. You've got a good point. We should be supporting a wider range of jQuery libraries. I remember the suggestion of us having ^1.8.0 || ^2.0.0, which I think should handle the cases we need, but apparently it never made its way in!

Here's an argument for the caret operator:

1.8 is the minimum that we need. 1.10 mirrors 2.0 in terms of functionality, only the latter targets modern browsers exclusively. The jQuery team has stated that they will definitely follow semver. Conclusion: ^1.8.0 || ^2.0.0

@thejameskyle's flipping of tables shouldn't be interpreted to mean that this isn't an important issue :)

@jamesplease jamesplease reopened this Jul 19, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling a8a5d35 on scotje:bower_semver_ranges into be4cd62 on marionettejs:master.

@scotje
Copy link
Author

scotje commented Jul 19, 2014

Ha, OK. You guys know your actual dependencies, it just seemed odd to accept 2.1.0 but no further down the 2.1.x branch. :) Trust me that I know this stuff sucks, especially when disjoint ranges come into play.

@jamesplease
Copy link
Member

Heh heh. Yeah. Hopefully this will resolve it once and for all!

@marionettejs/marionette-core, what do you guys think about ^1.8.0 || ^2.0.0?

@jamiebuilds
Copy link
Member

@scotje I'm so sorry! I was joking around!

@scotje
Copy link
Author

scotje commented Jul 19, 2014

Haha, no worries guys.

On Jul 19, 2014, at 9:01, James Kyle notifications@github.com wrote:

@scotje I'm so sorry! I was joking around!


Reply to this email directly or view it on GitHub.

@scotje
Copy link
Author

scotje commented Jul 20, 2014

I can fixup this PR with the new jQuery range. Was there any reason that backbone and/or underscore were capped on the upper end the way they were?

@jamesplease
Copy link
Member

Yup, they've chosen not to follow semver so even a patch release could be breaking. We have to cap it so that folks' apps don't break when they update dependencies. On your own apps I recommend that you also hardcore the version of those libs :)

@samccone
Copy link
Member

I am ok for increasing the jquery range

Some of the previous ranges didn't really make sense and seemed unintentionally restrictive. (e.g. Allowing jQuery 2.1.0 but no other 2.1.x releases.)
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 43aff31 on scotje:bower_semver_ranges into 0165ba2 on marionettejs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 43aff31 on scotje:bower_semver_ranges into 0165ba2 on marionettejs:master.

@scotje
Copy link
Author

scotje commented Jul 21, 2014

OK, I updated the PR to include only the jQuery version range change.

@rhubarbselleven
Copy link
Contributor

Older versions of npm do not support the caret operator. Is there a similar support issue with bower?

@jamesplease
Copy link
Member

They both use node-semver, so yup. I'm not sure what the lowest supported version is, though.

@Anachron
Copy link

The new dependency statement looks good to me. 👍

@samccone
Copy link
Member

~ .x.x is what we want for lower versions of npm and bower

@scotje
Copy link
Author

scotje commented Jul 23, 2014

I just want to clarify, is anything from the 1.x series (that is greater than 1.8.0) acceptable? Or only 1.8.x? If it's everything after 1.8.0, it seems like the only actual requirements here are ">=1.8.0 <3.0.0". As long as you guys trust jQuery adherence to semver.

@Anachron
Copy link

@scotje please read my post here:
#1303 (comment)

@samccone
Copy link
Member

Hi @scotje thanks for raising it, all resolved in #1778

@samccone samccone closed this Aug 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants