Replace go-bindata with //go:embed from Go 1.16 #392
Conversation
| ui/.build/ui: node_modules $(wildcard ui/assets/**/*) | ||
| mkdir -p ui/.build | ||
| rsync -r ui/assets/* ui/.build/ui | ||
| npx webpack $(WEBPACK_ARGS) | ||
| $< -o $@ -pkg ui -prefix .build/ui -nomemcopy .build/ui/... |
There was a problem hiding this comment.
Maybe this rule needs some tuning for proper rebuilds.
There was a problem hiding this comment.
Can you elaborate more on what you mean here?
There was a problem hiding this comment.
I'll try. Since ui/.build/ui is a directory, I'm not sure under which circumstances make will figure out that the target needs to be rebuilt. Maybe it would be better to only check the JavaScript bundle generated by webpack?
simon04
force-pushed
the
go-embed
branch
2 times, most recently
from
8e80da4 to
607adc0
Compare
| jobs: | ||
| go-build: | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| go: ["1.14", "1.13", "1.12"] | ||
| go: ["1.16"] |
There was a problem hiding this comment.
While it's true that we probably don't need to be supporting Go 1.12 or 1.13, I'm wary of dropping support for everything but the latest version. I believe the two most recent Go versions are officially supported. This might make a little more sense once Go 1.17 is out.
There was a problem hiding this comment.
Walking around and reminding that Go 1.17 was released.
| rsync -r ui/assets/* .build/ui | ||
| ui/.build/ui: node_modules $(wildcard ui/assets/**/*) | ||
| mkdir -p ui/.build | ||
| rsync -r ui/assets/* ui/.build/ui | ||
| npx webpack $(WEBPACK_ARGS) |
There was a problem hiding this comment.
I think the original motivation around using bindata in the first place is so that we could ensure built static assets were present and usable immediately after go install, without requiring an additional build step / installing node_modules / etc. I'm admittedly fuzzy about whether or not Go Modules have a post-build lifecycle step, but I think that context is important for this PR.
There was a problem hiding this comment.
I have had success with pkger.
|
replaced by #430 |
Fixes #381.
cc: @salemhilal