Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

smalloc: remove module #3099

Closed

Conversation

brendanashworth
Copy link
Contributor

Bye bye, smalloc. I'm not sure why this was still here; it was removed
a little while ago and hasn't worked since. It wasn't packaged in the
binary, either.

$ node
> process.version
'v4.1.1'
> require('smalloc')
Error: Cannot find module 'smalloc'
    ...

Bye bye, smalloc. I'm not sure why this was still here; it was removed
a little while ago and hasn't worked since. It wasn't packaged in the
binary, either.
@targos
Copy link
Member

targos commented Sep 28, 2015

LGTM if the tests pass.

@thefourtheye
Copy link
Contributor

Exactly the same as targos's comment

@brendanashworth
Copy link
Contributor Author

@ChALkeR
Copy link
Member

ChALkeR commented Sep 28, 2015

LGTM.

For reference, require\(.smalloc usage in npm packages:

Copy-paste from node.js/io.js:

biojs-vis-blast-0.1.5.tgz/node/lib/fs.js:43:var kMaxLength = require('smalloc').kMaxLength;
biojs-vis-blast-0.1.5.tgz/node/test/simple/test-buffer.js:27:var smalloc = require('smalloc');
biojs-vis-blast-0.1.5.tgz/node/test/simple/test-smalloc.js:27:var smalloc = require('smalloc');
flush-all-0.1.1.tgz/node-v0.13/lib/fs.js:43:var kMaxLength = require('smalloc').kMaxLength;
flush-all-0.1.1.tgz/node-v0.13/test/simple/test-buffer.js:27:var smalloc = require('smalloc');
flush-all-0.1.1.tgz/node-v0.13/test/simple/test-smalloc.js:27:var smalloc = require('smalloc');
jsg-0.0.3.tgz/testdata/node_core_modules/fs.js:41:var kMaxLength = require('smalloc').kMaxLength;
mock-fs-3.2.0.tgz/node/fs-0.11.13.js:41:var kMaxLength = require('smalloc').kMaxLength;
mock-fs-3.2.0.tgz/node/fs-0.12.0.js:43:var kMaxLength = require('smalloc').kMaxLength;
mock-fs-3.2.0.tgz/node/fs-1.1.0.js:20:const kMaxLength = require('smalloc').kMaxLength;
mock-fs-3.2.0.tgz/node/fs-2.0.0.js:20:const kMaxLength = require('smalloc').kMaxLength;
nae-fs-0.0.2.tgz/_source/fs.js:42:var kMaxLength = require('smalloc').kMaxLength;
node-core-lib-0.11.11.tgz/fs.js:41:var kMaxLength = require('smalloc').kMaxLength;
node-core-test-simple-0.11.11.tgz/test-smalloc.js:26:var smalloc = require('smalloc');
portable-js-0.0.3.tgz/misc/node/fs.js:45:var kMaxLength = require('smalloc').kMaxLength;

Other usage:

nodeinjection-0.0.5.tgz/src/CoreModules.js:26:    registry.register('smalloc', [], function() { return require('smalloc'); });
reblaze-0.2.1.tgz/bench/matrix.js:1:var smalloc = require('smalloc');

@bnoordhuis
Copy link
Member

LGTM. I forgot to remove it in 70d1f32 although I did remove lib/internal/smalloc.js.

@Fishrock123
Copy link
Contributor

This will currently throw an error anyways, right? That would mean this can land in 4.x?

@ChALkeR
Copy link
Member

ChALkeR commented Sep 28, 2015

@Fishrock123 Yes.

@brendanashworth
Copy link
Contributor Author

@Fishrock123 no harm - not sure if there'd be a benefit of back porting it, though - it just clears confusion on master.

edit: ci is happy, merging

brendanashworth added a commit that referenced this pull request Sep 28, 2015
Bye bye, smalloc. I'm not sure why this was still here; it was removed
in 70d1f32 and hasn't worked since. It wasn't packaged in the
binary, either.

Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #3099
@brendanashworth
Copy link
Contributor Author

Landed in 20dae2a, thank you reviewers!

brendanashworth added a commit that referenced this pull request Sep 30, 2015
Bye bye, smalloc. I'm not sure why this was still here; it was removed
in 70d1f32 and hasn't worked since. It wasn't packaged in the
binary, either.

Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #3099
This was referenced Sep 30, 2015
brendanashworth added a commit that referenced this pull request Sep 30, 2015
Bye bye, smalloc. I'm not sure why this was still here; it was removed
in 70d1f32 and hasn't worked since. It wasn't packaged in the
binary, either.

Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #3099
@MylesBorins
Copy link
Contributor

landed in v4.x-staging in 37cdeaf

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants