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

use go:embed to bind data #156

Merged
merged 1 commit into from
Jan 3, 2023
Merged

Conversation

xwjdsh
Copy link
Contributor

@xwjdsh xwjdsh commented Jan 23, 2022

Simplify code.

@jhump
Copy link
Contributor

jhump commented Feb 14, 2022

@xwjdsh, thanks for the patch!

We intentionally support Go runtimes older than G 1.17. After 1.18 is GA (which should be in the coming month), maybe we can re-assess since, IIUC, the Go team will drop support for 1.16 at that point (based on their release policy).

In the meantime, let's leave this pull request open and consider merging it after that.

@xwjdsh
Copy link
Contributor Author

xwjdsh commented Feb 14, 2022

@jhump actually the embed feature added in go1.16, I can change the version of the go.mod to go.1.16 if you want.

@dragonsinth
Copy link
Member

This would need a rebase for conflict

@xwjdsh
Copy link
Contributor Author

xwjdsh commented Dec 22, 2022

@dragonsinth Updated.

@dragonsinth
Copy link
Member

https://github.com/fullstorydev/grpcui/pull/156/files#diff-288b5fde448529a95912cdc56f6eddfbb5baaff11e11bf53c3d1e3efed7c91b5R57

maybe // Add embedded resources bundled in standalone package TOC?

go1. 17 (released 2021-08-16)
@jhump seems reasonable to support go 17, 18, 19 to me.

@jhump
Copy link
Contributor

jhump commented Jan 3, 2023

@jhump seems reasonable to support go 17, 18, 19 to me.

Sure, seems fine to me. I had commented almost a year ago, before 1.18 was released.

@dragonsinth dragonsinth merged commit 916a49f into fullstorydev:master Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants