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

DO_NOT_MERGE test: system test node 14 and 16 #851

Closed
wants to merge 2 commits into from
Closed

DO_NOT_MERGE test: system test node 14 and 16 #851

wants to merge 2 commits into from

Conversation

amchiclet
Copy link
Contributor

No description provided.

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: cloudprofiler Issues related to the googleapis/cloud-profiler-nodejs API. labels Aug 15, 2022
@codecov
Copy link

codecov bot commented Aug 15, 2022 •
edited
Loading

Codecov Report

Merging #851 (c01f213) into main (f73d713) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #851   +/-   ##
=======================================
  Coverage   68.87%   68.87%           
=======================================
  Files           7        7           
  Lines        1253     1253           
  Branches       58       58           
=======================================
  Hits          863      863           
  Misses        389      389           
  Partials        1        1

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@amchiclet amchiclet added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 15, 2022
@amchiclet amchiclet marked this pull request as ready for review August 15, 2022 18:27
@amchiclet amchiclet requested review from a team as code owners August 15, 2022 18:27
@amchiclet amchiclet added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 15, 2022
@nolanmar511
Copy link
Contributor

LGTM; possible that Node 18 tests will fail b/c there aren't prebuilt binaries for Node 18.

@amchiclet amchiclet added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 15, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 15, 2022
@amchiclet
Copy link
Contributor Author

Yep, you're right. Pre-built is an issue for node 18. Scoping this PR to only include node 16.

@nolanmar511 nolanmar511 changed the title DO_NOT_MERGE test: system test node 14, 16, and 18 DO_NOT_MERGE test: system test node 14 and 16 Aug 16, 2022
@weyert
Copy link

weyert commented Oct 24, 2022

Any news on the status of this PR? Tomorrow Node v18 will become LTS and so far we haven't been able to use Profiling while I think it can be really handy to have when trying to get to the bottom of performance issues on GKE

@amchiclet
Copy link
Contributor Author

There's work being done now that is needed to support Node 18 in pprof-nodejs (which this project depends on).

After that work is done, this project will then be updated to support up to Node 18. Unfortunately I don't have the capacity to continue this work and somebody else in the team will be picking it up.

@weyert
Copy link

weyert commented Oct 24, 2022

There's work being done now that is needed to support Node 18 in pprof-nodejs (which this project depends on).

After that work is done, this project will then be updated to support up to Node 18. Unfortunately I don't have the capacity to continue this work and somebody else in the team will be picking it up.

Great, glad to see, work is in progress to get support for newer versions of Node :)

@aabmass
Copy link
Collaborator

aabmass commented Feb 2, 2024

This was already addressed in #891

I will also add node 20 in that file now

@aabmass aabmass closed this Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nolanmar511 nolanmar511 nolanmar511 approved these changes

Assignees
No one assigned
Labels
api: cloudprofiler Issues related to the googleapis/cloud-profiler-nodejs API. do not merge Indicates a pull request not ready for merge, due to either quality or timing. size: s Pull request size is small.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@amchiclet @nolanmar511 @weyert @aabmass @kokoro-team