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

Update mocha, chai, chai-as-promised deps. #15

Merged
merged 1 commit into from
Jul 27, 2017
Merged

Conversation

mattcollier
Copy link
Contributor

I propose this would be a minor version bump.

@dlongley
Copy link
Member

@mattcollier,

I propose this would be a minor version bump.

Why not just a patch? What's new that is exposed by bedrock? Are mocha and chai exposed in tests outside of the module without those other modules including mocha and chai on their own? If so, we should remedy this in the future (potentially with a bedrock-mocha package as discussed elsewhere).

Copy link
Member

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

LGTM.

@mattcollier
Copy link
Contributor Author

@dlongley minor because we're doing major upgrades on 3 dependencies. I imagine there is some breaking change in there somewhere (although I don't know of any specifics). Seems to me that in a case like this, being able to control an upgrade via semver is desirable.

@dlongley
Copy link
Member

@mattcollier,

If those dependencies are only used internally, however, it shouldn't matter. If bedrock's behavior via its own API has changed in no way, there's no reason to do a minor version bump.

@mattcollier mattcollier merged commit fb15caf into master Jul 27, 2017
@dlongley
Copy link
Member

So the mocha and chai dependencies listed in this library are exposed as globals in tests -- including for modules that depend on bedrock, making this a tricky situation. We do not want to have to do major version releases of bedrock because testing infrastructure has been updated because it's unnecessarily disruptive. Given that bedrock exposes mocha and chai's API, a major version bump to those libraries should cause a major bump for bedrock as well, which is quite undesirable.

We're going to compromise and do a minor version release of bedrock -- and file an issue to move the mocha-specific test framework into a bedrock-mocha package to avoid this issue in the future.

@mattcollier mattcollier deleted the updateTestDeps branch July 27, 2017 15:43
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