Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

overview: show git branch #346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

overview: show git branch #346

wants to merge 1 commit into from

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Feb 10, 2020

I think that this should also show the git branch in the overview page (if available), but I haven't tested fully. It does correctly add the branch to the report.json when I run klab report locally.

@asymmetric did you ever find a way to test this locally?

Also snuck in a few lil unrelated stylistic changes to maximize bikeshedding potential 😇

  • renamed proj to rev
  • truncated the git hash to the first 8 characters

@@ -444,7 +445,8 @@ const json_report = {
[build_hash]: {
date: new Date(),
proofs,
git: project_HEAD
git: project_HEAD,
branch: getProjectBranch()
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like we could start adding these things to a git key, rather than having multiple keys. git.head = ...; git.branch = ..., maybe git.url in the future.

Copy link
Contributor Author

@d-xo d-xo Feb 18, 2020

Choose a reason for hiding this comment

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

this is a good idea, but I wonder if we will run into issues with backwards compatibility with old reports?

Copy link
Contributor

Choose a reason for hiding this comment

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

Where would this break? You'd need to change in overview.js as well ofc. Are there other coupling points?

Anyway, this doesn't have to hold back this PR.

resources/overview.js Show resolved Hide resolved
@asymmetric
Copy link
Contributor

Nope, didn't find a way to test this locally, I just created it on the box and copied it by hand.

You could use #342 now, which I'll rebase and merge promptly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants