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

Added version to web interface. #195

Merged
merged 3 commits into from
May 5, 2021

Conversation

Harnish
Copy link
Contributor

@Harnish Harnish commented Apr 29, 2021

I broke this out to its own little change since I wanted to get your feedback on using the makefile to put the version into server.go vs passing it along.

I am trying to decom my project https://github.com/Harnish/https-dns-proxy and move the fun bits into blocky.

@codecov
Copy link

codecov bot commented Apr 29, 2021

Codecov Report

Merging #195 (1d6ab01) into development (dd69a3e) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #195      +/-   ##
===============================================
+ Coverage        93.79%   93.82%   +0.02%     
===============================================
  Files               27       27              
  Lines             1741     1749       +8     
===============================================
+ Hits              1633     1641       +8     
  Misses              80       80              
  Partials            28       28              
Impacted Files Coverage Δ
cmd/root.go 70.37% <ø> (ø)
cmd/serve.go 66.66% <100.00%> (ø)
cmd/version.go 100.00% <100.00%> (ø)
server/server_endpoints.go 87.50% <100.00%> (+0.78%) ⬆️

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 dd69a3e...1d6ab01. Read the comment docs.

@0xERR0R
Copy link
Owner

0xERR0R commented Apr 29, 2021

Hi,
thanks for the suggestion. The idea is good to display the version number in the initial web page. Currently, blocky uses the version number in 2 places:

  • Server startup: shows the version in the banner (serve.go, will be injected in the makefile)
  • Prometheus metric (metric_event_publisher.go, receives the version number via application event over the event bus. The event will be sent in serve.go - startServer func)

I think, it would be better to receive the event in Server (same logic as in metric_event_publisher) via event bus and not to duplicate the injection in the make file.

Another approach would be to move the version number to a common place and inject it only once.

What do you think?

@0xERR0R
Copy link
Owner

0xERR0R commented May 5, 2021

I moved version and build time to the util package (to avoid duplication of variables) and tested your PR -> looks good. I added also build time to the web interface, so the web UI shows the same information as the log file.

Thanks for you work!

@0xERR0R 0xERR0R added the 🔨 enhancement New feature or request label May 5, 2021
@0xERR0R 0xERR0R added this to the 0.15 milestone May 5, 2021
@0xERR0R
Copy link
Owner

0xERR0R commented May 5, 2021

Closes #203

@0xERR0R 0xERR0R merged commit d8903bc into 0xERR0R:development May 5, 2021
@0xERR0R 0xERR0R linked an issue May 6, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show version number and build time in the web interface
2 participants