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

Simplify Golang images #416

Merged
merged 5 commits into from
Oct 28, 2020
Merged

Conversation

briandealwis
Copy link
Member

@briandealwis briandealwis commented Aug 17, 2020

As Cloud Code for VS Code now leverages skaffold debug under the hood, this PR re-proposes #102 with some slight updates.

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 17, 2020
COPY index.html /app/index.html
COPY assets /app/assets/

ENTRYPOINT ["/app/server"]

Choose a reason for hiding this comment

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

Why do we change the entrypoint from /app to /app/server?

Copy link
Member Author

Choose a reason for hiding this comment

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

It felt wrong to be copying index.html and assets into the root. It also seemed weird to have /app/app.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm more than happy to change /app/server to something else. /deploy/app?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed this to /hello-world and kept the binary as app.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM - @simonz130 any additional feedback?

@briandealwis
Copy link
Member Author

/gcbrun

@donmccasland
Copy link
Member

/gcbrun

@kelsk
Copy link
Contributor

kelsk commented Oct 27, 2020

/gcbrun

@kelsk kelsk merged commit 494e8fb into GoogleCloudPlatform:master Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants