-
Notifications
You must be signed in to change notification settings - Fork 903
feat: add version to footer #882
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
Conversation
For now it just returns the version.
Partially addresses #376.
Codecov Report
@@ Coverage Diff @@
## main #882 +/- ##
==========================================
- Coverage 66.04% 66.00% -0.05%
==========================================
Files 218 220 +2
Lines 13849 13890 +41
Branches 103 105 +2
==========================================
+ Hits 9147 9168 +21
- Misses 3783 3796 +13
- Partials 919 926 +7
Continue to review full report at Codecov.
|
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.
Go side looks good to me. Just a few minor things! 😎
Moving to a draft since I neglected to update the frontend footer test. |
The rest of the file is not alphabetic but might as well.
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.
This looks great to me. Had some fairly low-key suggestions around conditionally rendering and using a function to construct the version string; nothing major.
Am going to look out for @presleyp 's review to see if there's any suggestions on the xService or global context 🙌🏻 .
@@ -1,7 +1,7 @@ | |||
import { screen } from "@testing-library/react" | |||
import React from "react" | |||
import { MockBuildInfo, render } from "../../test_helpers" | |||
import { Footer } from "./Footer" | |||
import { render, MockBuildInfo } from "../../test_helpers" |
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 swear render
and MockBuildInfo
keep shuffling on their own but that seems impossible. 👽
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 empathize with this feeling
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.
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.
LGTM! Nice change.
Looks like one line got one character too long after the merge.
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.
Can you merge latest main into this branch so that we get the Chromatic workflow to run
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.
Nicely done!
Uh oh!
There was an error while loading. Please reload this page.