sourcegraph / appdash

Application tracing system for Go, based on Google's Dapper.
https://sourcegraph.com
Other
1.72k stars 137 forks source link

Use vfsgen to statically embed template data. #84

Closed dmitshur closed 9 years ago

dmitshur commented 9 years ago

Reasons for switching to vfsgen include:

dmitshur commented 9 years ago

I've tried to keep the filenames unchanged to make it easier to review.

traceapp/tmpl/data_src.go is still the // +build dev live disk reload version, but it's a much smaller file now because it simply uses http.Dir. It defines an Assets var, and that's what's used in dev mode, as well as by vfsgen during go generate to generate data.go file.

traceapp/tmpl/data.go is still the // +build !dev go generated static embedded version, but it's now generated with vfsgen.

dmitshur commented 9 years ago

Travis CI is failing, but not sure if that's related to my changes or external. The last master build was successful, but it was 12 days ago. Can someone with access restart CI to check if master is still passing?

dmitshur commented 9 years ago

It turns out Travis is passing on master even now, so this appears to be a legitimate failure due to my changes in this PR. I'll try to see what the cause could be.

slimsag commented 9 years ago

@shurcooL I suspect perhaps the filepaths have changed somehow, but can't say for certain

slimsag commented 9 years ago

On second thought, this appears unrelated to your change. Running master locally (go test -v ./...) produces the same test failures for me.

dmitshur commented 9 years ago

The Makefile that was in the root folder previous to being removed in this PR was causing Travis not to run tests at all, that's why it succeeded.

See the bottom of https://travis-ci.org/sourcegraph/appdash/jobs/79379987 output:

The command "make" exited with 0.

Done. Your build exited with 0.

slimsag commented 9 years ago

@shurcooL I've merged #85 in, please rebase so we can see whether or not that fixed the issue :)

dmitshur commented 9 years ago

Done, and it's green now.

slimsag commented 9 years ago

LGTM