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

Upgrade nise and browserify #2175

Merged
merged 8 commits into from
Dec 19, 2019
Merged

Upgrade nise and browserify #2175

merged 8 commits into from
Dec 19, 2019

Conversation

mantoni
Copy link
Member

@mantoni mantoni commented Dec 18, 2019 •
edited
Loading

Purpose (TL;DR) - mandatory

  • Upgrade nise to v2 which includes lolex v5 so that we don't have two lolex versions in the tree.
  • Use mochify --no-detect-globals which needed a few adjustments to global detection in test cases.
  • Upgrade browserify to latest
  • Set detectGlobals to false when bundling Sinon.

The last step currently fails the Worker test because we're doing the global detection wrong everywhere. I suggest we create a utility for this in commons, use that in nise and lolex and create patch releases. Edit: this has been done.

Fixes #2119

How to verify - mandatory

  1. Check out this branch
  2. npm install
  3. npm test
  4. npm run test-cloud

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached.

@mantoni
Copy link
Member Author

mantoni commented Dec 18, 2019

PR to have a common way to detect the correct global: sinonjs/commons#46

This was referenced Dec 18, 2019
Merged
Merged
@mantoni mantoni force-pushed the upgrade-nise-and-browserify branch from 6705b76 to f34a13e Compare December 19, 2019 12:34
mantoni
mantoni commented Dec 19, 2019
View reviewed changes
build.js
"./lib/sinon.js",
{
// Create a UMD wrapper and install the "sinon" global:
standalone: "sinon",
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this wasn't done before, which makes this bundle essentially unusable. I guess nobody actually uses this.

@mantoni mantoni marked this pull request as ready for review December 19, 2019 12:48
@mantoni mantoni changed the title [WIP] Upgrade nise and browserify Upgrade nise and browserify Dec 19, 2019
@mantoni mantoni requested review from mroderick and fatso83 December 19, 2019 12:49
@mantoni mantoni added the semver:major changes will cause a new major version label Dec 19, 2019
mroderick
mroderick approved these changes Dec 19, 2019
View reviewed changes
Copy link
Member

@mroderick mroderick left a comment

Choose a reason for hiding this comment

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

This looks good to me 👍

@mantoni mantoni merged commit 6438640 into master Dec 19, 2019
@mantoni mantoni deleted the upgrade-nise-and-browserify branch December 19, 2019 16:11
@mantoni mantoni mentioned this pull request Dec 20, 2019
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroderick mroderick mroderick approved these changes

@fatso83 fatso83 Awaiting requested review from fatso83

Assignees
No one assigned
Labels
semver:major changes will cause a new major version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mantoni @mroderick