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

Fix helloworld GCF test #739

Merged
merged 10 commits into from
Sep 25, 2018
Merged

Fix helloworld GCF test #739

merged 10 commits into from
Sep 25, 2018

Conversation

ace-n
Copy link
Contributor

@ace-n ace-n commented Sep 12, 2018

Do not merge until functions/helloworld tests pass.

@ace-n ace-n added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 12, 2018
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 12, 2018
@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Sep 12, 2018
@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 12, 2018
@GoogleCloudPlatform GoogleCloudPlatform deleted a comment from googlebot Sep 12, 2018
@GoogleCloudPlatform GoogleCloudPlatform deleted a comment from googlebot Sep 12, 2018
@ace-n
Copy link
Contributor Author

ace-n commented Sep 17, 2018

Note to Googlers: this PR fixes b/114412909

@ace-n ace-n requested a review from fhinkel September 17, 2018 23:02
@ace-n ace-n removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 24, 2018
fhinkel
fhinkel reviewed Sep 24, 2018
View reviewed changes
.kokoro/build.sh
@@ -32,6 +34,10 @@ gcloud config set project $GCLOUD_PROJECT

# Start functions emulator, if appropriate
if [[ $PROJECT == functions/* ]]; then
export FUNCTIONS_LOG_PATH=$(pwd)/logs/cloud-functions-emulator.log
npm install -g @google-cloud/functions-emulator
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way we could avoid global installs? Would npx solve that for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps - however, these containers are (AFAIK) ephemeral. Is making (and potentially debugging) this change a good use of time?

Copy link
Contributor

Choose a reason for hiding this comment

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

If anybody wants to reproduce this locally, they might end up with global installs which is not nice. Also, problem caused by globally installed dependencies are hard to debug.

It should be enough to delete the npm i -g line and further down call:
npx functions-emulator start (instead of functions-emulator start).

Let's land it as is to figure out if it works. And then do another revision to get rid of the global installs.

@ace-n
Copy link
Contributor Author

ace-n commented Sep 25, 2018

@dpebot merge when green

@dpebot
Copy link
Contributor

dpebot commented Sep 25, 2018

Okay! I'll merge when all statuses are green and all reviewers approve.

@dpebot dpebot added the automerge Merge the pull request once unit tests and other checks pass. label Sep 25, 2018
@dpebot dpebot self-assigned this Sep 25, 2018
@ace-n ace-n added the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 25, 2018
@ace-n ace-n merged commit a29f65f into master Sep 25, 2018
@ace-n ace-n deleted the gcf-kokoro-2 branch September 25, 2018 20:55
ahrarmonsur pushed a commit that referenced this pull request Nov 18, 2022
@b-loved-dreamer
chore: removed singleutterance flag (#739)
b9f2fd0
ahrarmonsur pushed a commit that referenced this pull request Nov 18, 2022
@b-loved-dreamer
chore: removed singleutterance flag (#739)
0e1e4f0
@ahrarmonsur ahrarmonsur mentioned this pull request Nov 18, 2022
Merged
NimJay pushed a commit that referenced this pull request Nov 18, 2022
@b-loved-dreamer @NimJay
chore: removed singleutterance flag (#739)
e368557
This was referenced Jun 20, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhinkel fhinkel fhinkel approved these changes

Assignees

@dpebot dpebot

Labels
automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement. kokoro:run Add this label to force Kokoro to re-run the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ace-n @dpebot @fhinkel @googlebot