-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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.
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
Conformance tests for JavaScript (Node.js). 15 tests are failing. #1662
Conversation
retest this please |
|
||
/* | ||
* Protocol Buffers - Google's data interchange format | ||
* Copyright 2008 Google Inc. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2016?
@haberman high level comment: can you please provide more context on what this change is trying to accomplish? I'm going into this without any background and a summary would help. A top-level comment in the conformance test wouldn't be a bad idea. |
conformance/conformance_nodejs.js
Outdated
function readBuffer(bytes) { | ||
var buf = new Buffer(bytes); | ||
var totalRead = 0; | ||
//console.warn("Want to read: " + bytes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to delete these commented out console.warn
statements.
Could you perhaps add some short instructions to conformance/README.md about how to run the new tests? Otherwise LGTM! |
Addressed your comments, PTAL. Thanks! |
LGTM |
Review to @joeltine.