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.

Sign up for GitHub

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

Jump to bottom

Avoid updating to 2.0.0 jasmine-reporters #333

Closed
wants to merge 2 commits into from
Closed

Avoid updating to 2.0.0 jasmine-reporters #333

wants to merge 2 commits into from

Conversation

Maxim-Filimonov
Copy link

Quick fix for #330
Have confirmed that bug doesn't occur on 1.0.0

@sembrestels
Copy link

+1 It is happening the same to me and it is working with the patch.

@Maxim-Filimonov
Copy link
Author

#331 & #332 - other versions of fix for the same problem. This issue does break a lot of projects depending on jasmine-node. Hope one of them get merged soon.

@ljharb
Copy link

ljharb commented Jul 7, 2014

Better would be to not use >= in code, and instead use ~ to only pull in patch versions.

@Maxim-Filimonov
Copy link
Author

@ljharb than it would not be possible to use v.1.0.0 I believe

@ljharb
Copy link

ljharb commented Jul 7, 2014

Then it should be locked to ~1.0.0. Dependencies shouldn't be allowed to change major versions implicitly.

@edi9999
Copy link

edi9999 commented Jul 8, 2014

As ljharb says, you should use ^1.0

@ljharb
Copy link

ljharb commented Jul 8, 2014

Please do not use the ^ though - that will make node 0.8 and earlier break. Stick with the ~

@Maxim-Filimonov
Copy link
Author

Right good catch, forgot about old node versions.
See updated version

This was referenced Jul 8, 2014
Closed
Closed
@jsongHBO
Copy link

jsongHBO commented Jul 8, 2014

+1

@DavidTanner
Copy link

+1 This bug is currently breaking our builds. Any idea when this will be merged and released with a new version???

@putermancer putermancer mentioned this pull request Jul 11, 2014
Closed
@srosca
Copy link

srosca commented Jul 13, 2014

+1

@kacperus
Copy link

+1

PS. Please merge it soon.

@brianokeefe
Copy link

+1

@putermancer putermancer mentioned this pull request Jul 14, 2014
Closed
@thomasdashney
Copy link

+1 soon please!

@Maxim-Filimonov
Copy link
Author

@tebriel any chance of merging this anytime soon?

@ljharb ljharb mentioned this pull request Jul 15, 2014
Closed
@tebriel
Copy link
Contributor

tebriel commented Jul 16, 2014

Merged a different PR to fix. Published as 1.14.4, sorry y'all, new job has me putting out fires EVERYWHERE and all the time.

@tebriel tebriel closed this Jul 16, 2014
@ljharb
Copy link

ljharb commented Jul 16, 2014

@tebriel That was the wrong one - the carat character breaks older nodes (everything before 0.10.something). Please change it to use the tilde, like this PR has.

@ljharb ljharb mentioned this pull request Jul 16, 2014
Merged
@Maxim-Filimonov Maxim-Filimonov deleted the lock_jasmine_reporters branch July 16, 2014 19:51
Maxim-Filimonov added a commit to Maxim-Filimonov/rtd that referenced this pull request Jul 22, 2014
@Maxim-Filimonov
Update jasmine-node version
83bec37 
Jasmine-node fixed dependency issues with jasmine-reporters in the newer version
mhevery/jasmine-node#333
Needs this update otherwise npm install with latest 1.13 doesn't work :(
@Maxim-Filimonov Maxim-Filimonov mentioned this pull request Jul 22, 2014
Merged
@meertens meertens mentioned this pull request Aug 4, 2014
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.