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

Updated Jasmine-core to a dependency of version 3.2 #15

Merged
merged 5 commits into from
Aug 20, 2018

Conversation

footballencarta
Copy link
Contributor

Referencing #14 - jasmine 3.2 is now required for the latest version of the plugin to work.

Added it as a dependancy to allow projects to pick up the package difference and process accordingly.

@footballencarta footballencarta changed the title Updated Jasmine-core to a depenency of version 3.2 Updated Jasmine-core to a dependency of version 3.2 Aug 20, 2018
package.json Outdated
}
],
"dependencies": {
"jasmine-core": ">=3.2"
Copy link
Owner

@dfederm dfederm Aug 20, 2018

Choose a reason for hiding this comment

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

Isn't peerDependencies the right place to have this though? This package is a plugin, so typically you want all users of this package to explicitly have jasmine also in their package.json, rather than letting it be pulled in as a dependency of the plugin.

Relevant SO post: https://stackoverflow.com/a/34645112/7065632

Thoughts?

@footballencarta
Copy link
Contributor Author

No problem - file updated :)

Seems I've learnt something about peerDependancies today!

"resolved": "https://registry.npmjs.org/jasmine-core/-/jasmine-core-3.2.0.tgz",
"integrity": "sha512-35OOToo2lFczwZ/FdJkUOO/Gsp9FW7viWChYA7OgBfpjgTxbxmNKyNrGS3HHREHay5nJwJvu4RqAlvcBcCAWeA==",
"version": "3.2.1",
"resolved": "http://npm-packages.rmdev.zone/jasmine-core/-/jasmine-core-3.2.1.tgz",
Copy link
Owner

@dfederm dfederm Aug 20, 2018

Choose a reason for hiding this comment

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

Hmm, this doesn't seem right? Do you have a non-standard proxy set up or something? (Sorry I didn't notice this earlier)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we do - good spot, I didn't notice it either :)

@dfederm dfederm merged commit 1a9ea67 into dfederm:master Aug 20, 2018
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.

2 participants