Skip to content

Don't update npm when installing node#15491

Merged
murgatroid99 merged 1 commit intogrpc:masterfrom
murgatroid99:node_docker_npm_fix
May 22, 2018
Merged

Don't update npm when installing node#15491
murgatroid99 merged 1 commit intogrpc:masterfrom
murgatroid99:node_docker_npm_fix

Conversation

@murgatroid99
Copy link
Copy Markdown
Member

@murgatroid99 murgatroid99 commented May 21, 2018

Also add Node 10, because we are using that now too. I think this should permanently fix #15470.

@murgatroid99 murgatroid99 requested a review from jtattermusch May 21, 2018 18:58
@murgatroid99 murgatroid99 mentioned this pull request May 21, 2018
Merged
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

[microbenchmarks] No significant performance differences

jtattermusch
jtattermusch approved these changes May 21, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

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

LGTM.

@murgatroid99
Copy link
Copy Markdown
Member Author

murgatroid99 commented May 21, 2018

Rerunning tests because I forgot to push the images the first time

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 21, 2018 

[microbenchmarks] No significant performance differences

@murgatroid99
Copy link
Copy Markdown
Member Author

murgatroid99 commented May 22, 2018

Rerunning the tests again because this change impacts the interop dockerfiles, and the interop tests failed to run for other reasons.

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 22, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 22, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented May 22, 2018 

[microbenchmarks] No significant performance differences

@murgatroid99
Copy link
Copy Markdown
Member Author

murgatroid99 commented May 22, 2018

The interop cloud-to-cloud tests failed due to #15456. The modified Node dockerfiles were also tested in the cloud-to-prod tests.

@murgatroid99 murgatroid99 merged commit 18ce0cf into grpc:master May 22, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matt-kwong matt-kwong Awaiting requested review from matt-kwong

@nicolasnoble nicolasnoble Awaiting requested review from nicolasnoble

1 more reviewer

@jtattermusch jtattermusch jtattermusch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/build lang/node

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

grpc-node tests broken on master

4 participants

@murgatroid99 @grpc-testing @jtattermusch @kokoro-team