-
Notifications
You must be signed in to change notification settings - Fork 493
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
feat: add git revision #844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it!
In the future, we could make it more subtle, but right now, in this first iteration it may be a good idea to keep it more visible :)
We could even add "Beta" somewhere, just to set proper expectations.
Its better to undersell than oversell, Google's Gmail had "Beta" for years! 🙃
@olizilla I'd really like to have revision number present when I ship bundled ipfs-webui with a new beta companion. |
minor, but I'm trying to move us towards using consistent names for the app...
as an aside, I went looking for a more structured way to declare the git revision that a page was built from and found https://github.com/krallin/meta-revision which is listed on the WHATWG MetaExtensions proposals https://wiki.whatwg.org/wiki/MetaExtensions So we could do something like <meta name="generator" content="ipfs-webui https://github.com/ipfs-shipyard/ipfs-webui" />
<meta name="revision" content="[revision]" /> It's kinda mind blowing that there is no accepted structured way for a web page to declare what version it is. |
cooOooOoL. The e2e tests are failing because the robot is trying to click the settings link, but due to the default screen size that puppet opens pages at 800x600 the revision, the revision link appears over the settings link! now, my first instinct was to update the tests to use a large scree size, but I think this is actually our bug... @fsdiogo could you add a custom media query to hide the revision link where the screen height would mean that it would be covering the nav items. |
Nice catch @olizilla, I was wondering why the CI was always failing. It should be fixed now! |
Adds the git revision.
In a meta tag:
In the UI:
Closes #842.