-
Notifications
You must be signed in to change notification settings - Fork 291
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
v1.14/v1.15 includes a breaking change in a minor version. #433
Comments
k turns out it’s not in this project, but a dep: https://travis-ci.org/es-shims/es5-shim/builds/423137988 I’ll debug more later today; either the dep needs to get fixed or jasmine-node needs to pin it, whichever it is. |
Thanks for the update. From 1.14.x...master the only breaking change I can see is in "dependencies": {
- "coffee-script": ">=1.0.1",
+ "coffeescript": ">=1.0.1",
"jasmine-reporters": "~1.0.0" which has the same effect as using As a side point I would rather fix this one thing than revert the whole change in 7aa3b53. Less messy in the history (IMHO). Another side point is that there is already a "v2" branch (https://github.com/mhevery/jasmine-node/tree/Jasmine2.0) that seems to both have new features and be outdated. |
to avoid possibly breaking change in minor release (version 1.15.1) Ref: mhevery#433
to avoid potentially breaking change in a minor release (version 1.15.1) Ref: mhevery#433
to avoid potentially breaking change in a minor release and mark version 1.15.1 ref: mhevery#433
Some general comments: I would recommend transitioning from FYI both Another side point I discovered is that this project does not have anything like Travis CI enabled. Considering that this project is really in a "maintenance mode" I will probably leave this part at a very low priority. |
I just published version |
Thanks, I appreciate the effort - I'll post here if i think there's anything further for jasmine-node to do. |
Found the source: tj/node-growl#79 via jasmine-growl-reporter (AlphaHydrae/jasmine-growl-reporter#3) |
One possible change here would be to drop the jasmine-growl-reporter dep from ~1 back to ~0.2 - that seems to have been the trigger :-/ |
jasmine-node v1.14+ depends on ~1.0.1 of jasmine-growl-reporter: mhevery/jasmine-node#433 jasmine-growl-reporter v1.0.1 depends on growl ^1.10.2: AlphaHydrae/jasmine-growl-reporter#3 growl v1.10.1+ has a breaking change: tj/node-growl#79
@ljharb I would definitely see the following change in 7ad50c8 as a breaking change: --- a/package.json
+++ b/package.json
@@ -25,7 +25,7 @@
"dependencies": {
"coffee-script": ">=1.0.1",
"jasmine-reporters": "~1.0.0",
- "jasmine-growl-reporter": "~0.0.2",
+ "jasmine-growl-reporter": "~1.0.1",
"requirejs": ">=0.27.1",
"walkdir": ">= 0.0.1",
"underscore": ">= 1.3.1", I wish you would have reported this observation in a new, separate issue. Considering that you have already referenced this issue from some other places, I am am reopening it with updated title description to reflect the new information. I will try to resolve this one along with #435 (use tilde instead of carrot) in a patch today. And yes, you can definitely blame me for the breaking changes here😒 |
Thanks, since it was the same root problem I figured I'd post it here; i'm happy to file a new one if you like. |
Let's keep it here. You were right: evidence of the root problem was already given in the description. I think part of the confusion was that the breaking change in 7ad50c8 was in |
I would also like to see a quick, easy-to-read description of npm semver etiquette that we can point people to, to help avoid this kind of issue in the future. I just raised the request in semver/semver#461. |
It does not work on Node.js |
Should be fixed in recently published version @ljharb can you explain if there is any reason to continue supporting v1.x with support for Node.js pre-6.0? Is Node.js pre-4.0/pre-6.0 needed to test JavaScript shim? If so, would you guys be open to using something like Duktape to test shim of newer JavaScript versions? It would be ideal if we can drop and abandon support for Node.js 6.0 (in new major release). |
The shims need to be tested in older engines; duktape definitely wouldn't be feasible. It's totally fine if you want to drop support in v2; if it ever stops working, i'd have to migrate my tests off jasmine (i've already begun migrating all my packages that use mocha and jest off of them, for this reason). I'd rather not do that in this particular repo, though :-) |
Thanks; the issue is indeed fixed https://travis-ci.org/es-shims/es5-shim/builds/424538191 Much obliged!! |
Thanks for the confirmation. I've been thinking if |
v1.14 works on node < 4; v1.15 does not. This is a major change.
Please revert and publish a v1.15 patch; and if you'd like to drop node < 4, please republish as a v2.
See https://travis-ci.org/es-shims/es5-shim/jobs/423131852 for an example.
EDIT by @brodybits: the breaking change is in 7ad50c8:
See below and es-shims/es5-shim@5a27afb for explanation.
Second possibly breaking change was the following (already fixed now):
The text was updated successfully, but these errors were encountered: