-
Notifications
You must be signed in to change notification settings - Fork 290
fix: send streamingConfig as a separate write before audioContent #176
Conversation
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
streamingConfig: CONFIG, | ||
}); | ||
setImmediate(done); | ||
if (count === 0) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
Codecov Report
@@ Coverage Diff @@
## master #176 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 2 2
Lines 37 35 -2
=====================================
- Hits 37 35 -2
Continue to review full report at Codecov.
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
src/helpers.js
Outdated
payload.streamingConfig = config; | ||
through.obj((audioContent, _, next) => { | ||
if (audioContent !== undefined) { | ||
next(null, {audioContent}); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This one threw me for a loop as well, from what I can tell helpers main goal is to allow const stream = client.streamingRecognize();
stream.write({streamingConfig});
fs.createReadStream('my-file.wav').pipe(stream); as opposed to fs.createReadStream('my-file.wav')
.pipe(client.streamingRecognize({config})); |
@callmehiphop are you taking the PR over? That would be nice :) |
98c0f88
to
c4dcf52
Compare
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
@JustinBeckwith sure thing, just rebased and pushed a commit. I think we're good to go (unless we want another contributor to take a peak at it first 😄) |
073b4ae
to
e22ae4f
Compare
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) |
Fixes #173.
Fixes #182.
I personally am not fan of modifying test methods generated by gapic, since the gapic tests should test the underlying (not helper-wrapped) methods.
So I tried to just require the underlying source via
require('../src/v1')
, the tests would pass if it's not run along withhelpers.test.js
(npx mocha test/gapic-{v1|v1beta1}.js
). Somehow requiringhelpers.js
modifies the actual exports in the gapic generated libs - so when I runnpm test
it fails.