search

GoogleCloudPlatform / cloud-run-button

Let anyone deploy your GitHub repos to Google Cloud Run with a single click
https://cloud.run
Apache License 2.0
525 stars 91 forks source link

add --use-http2 flag support - fixes #202 #203

Closed jamesward closed 3 years ago

jamesward commented 3 years ago

Note that today the project must have the http2 alpha enabled.

ahmetb commented 3 years ago

One of the tests is failing:

=== RUN   TestGitCheckout
Error:     clone_test.go:110: git checkout failed: exit status 1, output:
        error: pathspec 'main' did not match any file(s) known to git

--- FAIL: TestGitCheckout (0.01s)

we can go merge but I'll try to investigate why.

ahmetb commented 3 years ago

Also somehow go1.15 fails to compile at 0e5f62b:

# github.com/GoogleCloudPlatform/cloud-run-button/cmd/cloudshell_open
cmd/cloudshell_open/deploy.go:158:10: conversion from int to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?)
jamesward commented 3 years ago

I believe the test is failing in Actions due to an older version of git where the default branch isn't main. Locally the test passes. Not sure how to workaround that.

Where are we using go1.15 ?

ahmetb commented 3 years ago

Where are we using go1.15 ?

on my development machine :) I pushed a commit fixing that.

we can t.Skip() the clone test maybe for the time being.

ahmetb commented 3 years ago

I believe we'll need to add

git config --global init.defaultBranch main

to the workflow configuration. I'll try that now.

jamesward commented 3 years ago

I added a param to git init which should explicitly set the branch to main (assuming the git version is high enough to have that flag)

ahmetb commented 3 years ago

I think the config outside the code is easier to cleanup later. I fixed that in 6a416b6 outside the code. (you now force-pushed over it.)

Also I think that particular test statement checking out to default branch isn't that useful. Maybe we can remove that, too.

jamesward commented 3 years ago

Whoops. Fun working on the same branch trying to fix the same thing. I'll let you take over :)

ahmetb commented 3 years ago

Yeah it seems both options were fine. Tests seem to be passing. We can merge.

jamesward commented 3 years ago

Cool. Thanks! Go ahead and merge it

ahmetb commented 3 years ago

I hope this gets picked up soon, it seems like we could use this in the announcement blog post.

jamesward commented 3 years ago

I can ask for a deploy

jamesward commented 3 years ago

This change is now deployed.