Skip to content

Conversation

@PatrickHeneise
Copy link

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Nov 6, 2017

fs.open(fn,'w',0o644,common.mustCall(function(err,fd){
assert.ifError(err);
console.log('open done');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these console statements can be left out.

Copy link
Author

Choose a reason for hiding this comment

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

done, thanks!

@mscdexmscdex added the fs Issues and PRs related to the fs subsystem / file system. label Nov 6, 2017
@gireeshpunathilgireeshpunathil added code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. and removed code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. labels Nov 6, 2017
fs.open(fn,'w',0o644,common.mustCall(function(err,fd){
assert.ifError(err);
constdone=common.mustCall(function(err,written){
assert.strictEqual(Buffer.byteLength(expected),written);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add an assert.ifError(err); here.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks

@PatrickHeneise
Copy link
Author

New PR without conflicts #17045

addaleax pushed a commit that referenced this pull request Nov 18, 2017
Easier way to resolve conflicts from #16822 and #16827. PR-URL: #17045 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Easier way to resolve conflicts from #16822 and #16827. PR-URL: #17045 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
Easier way to resolve conflicts from #16822 and #16827. PR-URL: #17045 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
Easier way to resolve conflicts from #16822 and #16827. PR-URL: #17045 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fsIssues and PRs related to the fs subsystem / file system.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@PatrickHeneise@mscdex@jasnell@cjihrig@gireeshpunathil@nodejs-github-bot