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

remove unneccessary Buffer require #13851

Closed
wants to merge 4 commits into from
Closed

remove unneccessary Buffer require #13851

wants to merge 4 commits into from

Conversation

lenakaplan
Copy link
Contributor

@lenakaplan lenakaplan commented Jun 21, 2017 •
edited
Loading

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 21, 2017
@vsemozhetbyt vsemozhetbyt added the buffer Issues and PRs related to the buffer subsystem. label Jun 21, 2017
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jun 21, 2017 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/8781/ (UPD: this is only for the first commit)

vsemozhetbyt
vsemozhetbyt reviewed Jun 21, 2017
View reviewed changes
test/parallel/test-buffer-nopendingdep-map.js Outdated
@@ -1,8 +1,7 @@
// Flags: --no-warnings --pending-deprecation
// Flags: --no-warnings --pending-deprecation
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this indent intentional?

vsemozhetbyt
vsemozhetbyt reviewed Jun 21, 2017
View reviewed changes
test/parallel/test-buffer-sharedarraybuffer.js Outdated
@@ -3,8 +3,6 @@
// Flags: --harmony-sharedarraybuffer

require('../common');
const assert = require('assert');
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this deletion is accidental.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lenakaplan Could you also undo this assert deletion so we can run a new CI on this PR?

benjamingr
benjamingr approved these changes Jun 21, 2017
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Nice work. Please feel free to also give us any feedback about the onboarding process today.

@refack refack mentioned this pull request Jun 21, 2017
Closed
@vsemozhetbyt
Copy link
Contributor

Thank you! New CI: https://ci.nodejs.org/job/node-test-pull-request/8799/

aqrln
aqrln approved these changes Jun 22, 2017
View reviewed changes
Copy link
Contributor

@aqrln aqrln left a comment

Choose a reason for hiding this comment

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

Thanks!

refack
refack approved these changes Jun 22, 2017
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

🥇

@benjamingr benjamingr mentioned this pull request Jun 22, 2017
Closed
@aqrln
Copy link
Contributor

aqrln commented Jun 24, 2017

Landed in 5e68e16. Congratulations and thanks for your first contribution!

@aqrln aqrln closed this Jun 24, 2017
aqrln pushed a commit that referenced this pull request Jun 24, 2017
@lenakaplan @aqrln
test: remove unnecessary require('buffer').Buffer
5e68e16 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Jun 24, 2017
@lenakaplan @addaleax
test: remove unnecessary require('buffer').Buffer
2f21db2 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax mentioned this pull request Jun 24, 2017
Merged
addaleax pushed a commit that referenced this pull request Jun 29, 2017
@lenakaplan @addaleax
test: remove unnecessary require('buffer').Buffer
ad9ebd5 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@lenakaplan @addaleax
test: remove unnecessary require('buffer').Buffer
dbdb1b5 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@lenakaplan @addaleax
test: remove unnecessary require('buffer').Buffer
df3c292 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
@lenakaplan @MylesBorins
test: remove unnecessary require('buffer').Buffer
036a97c 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@lenakaplan @MylesBorins
test: remove unnecessary require('buffer').Buffer
846c10c 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@lenakaplan @MylesBorins
test: remove unnecessary require('buffer').Buffer
cd962e6 
Remove unnecessary imports from:

* test/parallel/test-buffer-nopendingdep-map.js
* test/parallel/test-buffer-pending-deprecation.js
* test/parallel/test-buffer-sharedarraybuffer.js
* test/parallel/test-buffer-slow.js
* test/parallel/test-buffer-tojson.js
* test/parallel/test-buffer-zero-fill.js

Refs: #13836
PR-URL: #13851
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
Merged
This was referenced Sep 5, 2017
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

@benjamingr benjamingr benjamingr approved these changes

@cjihrig cjihrig cjihrig approved these changes

@aqrln aqrln aqrln approved these changes

Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@lenakaplan @vsemozhetbyt @aqrln @refack @jasnell @benjamingr @cjihrig @MylesBorins @nodejs-github-bot