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

Support embedding assets from a subdir #2846

Merged
merged 5 commits into from
Feb 26, 2021

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Feb 25, 2021

Part of #2749.

  • add util function for altering http.FileSystem behavior
  • refactor HotROD embedding to read from a subdir directly
  • replace esc with embedding for the main UI placeholder

@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #2846 (aca2af9) into master (0a34e46) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2846      +/-   ##
==========================================
+ Coverage   95.94%   95.96%   +0.02%     
==========================================
  Files         221      222       +1     
  Lines        9689     9696       +7     
==========================================
+ Hits         9296     9305       +9     
+ Misses        324      323       -1     
+ Partials       69       68       -1     
Impacted Files Coverage Δ
pkg/httpfs/prefixed.go 100.00% <100.00%> (ø)
cmd/query/app/static_handler.go 96.77% <0.00%> (+1.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a34e46...57063d2. Read the comment docs.

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro
Copy link
Member Author

@albertteoh fix for that nested dir

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Copy link
Contributor

@albertteoh albertteoh left a comment

Choose a reason for hiding this comment

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

nice! Looking forward to removing use of esc altogether; I noticed it's been a common question/problem from folks trying to run/build the backend components because they haven't installed it or $GOPATH isn't in their $PATH.

@yurishkuro yurishkuro merged commit c01aa2b into jaegertracing:master Feb 26, 2021
@yurishkuro yurishkuro deleted the embed_from_subdir branch February 26, 2021 02:26
@yurishkuro
Copy link
Member Author

Indeed. And the gen_assets kept being regenerated with only difference being the timestamp, pretty annoying.

We need to merge #2822 before the last esc piece can be removed.

@jpkrohling jpkrohling added this to the Release 1.23.0 milestone Jun 4, 2021
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.

3 participants