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

feat: add git revision #844

Merged
merged 3 commits into from
Oct 4, 2018
Merged

feat: add git revision #844

merged 3 commits into from
Oct 4, 2018

Conversation

fsdiogo
Copy link
Contributor

@fsdiogo fsdiogo commented Oct 2, 2018

Adds the git revision.

In a meta tag:

meta

In the UI:

ui

Closes #842.

@fsdiogo fsdiogo self-assigned this Oct 2, 2018
@fsdiogo fsdiogo requested review from olizilla and lidel October 2, 2018 13:57
Copy link
Member

@lidel lidel left a 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! 🙃

@lidel lidel mentioned this pull request Oct 3, 2018
9 tasks
@lidel
Copy link
Member

lidel commented Oct 3, 2018

@olizilla I'd really like to have revision number present when I ship bundled ipfs-webui with a new beta companion.
Please review when you have a spare moment :)

@olizilla
Copy link
Member

olizilla commented Oct 4, 2018

minor, but I'm trying to move us towards using consistent names for the app...

  • IPFS Web UI - it's human friendly name
  • ipfs-webui - it's machine friendly name

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.

@olizilla
Copy link
Member

olizilla commented Oct 4, 2018

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!

screenshot 2018-10-04 11 47 53

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.

@fsdiogo
Copy link
Contributor Author

fsdiogo commented Oct 4, 2018

Nice catch @olizilla, I was wondering why the CI was always failing.

It should be fixed now!

@olizilla olizilla merged commit 50e893e into master Oct 4, 2018
@olizilla olizilla deleted the feat/add-git-revision branch October 4, 2018 13:50
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