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.

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

fix: sample app TypeError #1257

Merged
merged 3 commits into from
May 7, 2020
Merged

Conversation

ZiweiZhao
Copy link
Contributor

App sample has error: TypeError: The GET method cannot be used with a body
This pull request is to update the body to an json response option

Fixes #1246 🦕

App sample has error: TypeError: The `GET` method cannot be used with a body
This pull request is to update the body to an json response option
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 4, 2020
@ZiweiZhao ZiweiZhao requested a review from kintel May 4, 2020 15:34
@bcoe
Copy link
Contributor

bcoe commented May 5, 2020

failing Windows tests are being discussed here:

nodejs/node-gyp#2114

@kintel
Copy link
Contributor

kintel commented May 6, 2020

@bcoe To move forward while the Node issues in question gets resolved (new Node version or new needle version), would it make sense to pin Node on Windows to 12.16.2 here? https://github.com/googleapis/cloud-trace-nodejs/blob/master/.github/workflows/ci.yaml#L61

@bcoe
Copy link
Contributor

bcoe commented May 6, 2020

@kintel yes, I would just switch to Node 10 on this branch for the purposes of landing. I believe the upstream issue will be fixed soon in Node.

@JustinBeckwith
Copy link
Contributor

Nice fix! One question though - are we missing an integration test for the sample? I'm a little surprised this didn't get captured in the end to end test.

@kintel
Copy link
Contributor

kintel commented May 7, 2020

@JustinBeckwith technically there is a test, but the test just checks that the process starts, not that it does the right thing. We should probably open a separate issue for improving the test.

@kintel
Copy link
Contributor

kintel commented May 7, 2020

Opened follow-up issue: #1259

@kintel kintel added the automerge Merge the pull request once unit tests and other checks pass. label May 7, 2020
@gcf-merge-on-green gcf-merge-on-green bot merged commit 1ac424e into googleapis:master May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

app sample broken: TypeError: The GET method cannot be used with a body
5 participants