Remove extra exports and use version from pkg.json
#819
Conversation
|
LGTM |
|
LGTM! |
|
|
||
| /*! | ||
| * Chai version | ||
| */ | ||
|
|
||
| exports.version = '3.5.0'; | ||
| exports.version = require('../package').version; |
There was a problem hiding this comment.
Just FYI our tooling automates writing this out as the string, so that we don't end up including all of the package.json in the browserify bundle.
There was a problem hiding this comment.
@keithamus Whoops. Revert? Also now that we have more maintainers/reviewers, maybe we should add a minimum time before merging a PR even after it has two approvals.
There was a problem hiding this comment.
OH,
Should we revert this?
Sorry for merging, should've waited for you.
There was a problem hiding this comment.
Yeah, when bundling for browsers result package size is an issue. We can use Rollup to decrease bundle size (tree-shaking and stuff) and include only the version field of package.json.
There was a problem hiding this comment.
@meeber @vieiralucas don't worry about reverting, our build process will write over this change when we release 4.0
@shvaikalesh We've already got the tooling that writes the version. We can definitely look at Rollup though; do you want to open an issue to discuss it?
There was a problem hiding this comment.
Also @meeber - I don't think we should add things like minimum time to review. This stuff happens, it's teething issues, but ultimately no big deal. Nothing egregious has happened - when it does we'll deal with it then!
No description provided.