-
Notifications
You must be signed in to change notification settings - Fork 1k
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
chore: Make Feast UI importable as a NPM Module #2370
chore: Make Feast UI importable as a NPM Module #2370
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2370 +/- ##
===========================================
- Coverage 85.47% 57.12% -28.35%
===========================================
Files 122 122
Lines 10714 10704 -10
===========================================
- Hits 9158 6115 -3043
- Misses 1556 4589 +3033
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
b5315b5
to
a378d82
Compare
|
||
interface FeaturesListProps { | ||
featureViewName: string; | ||
features: FeastFeatureColumnType[]; | ||
} | ||
|
||
const FeaturesList = ({ featureViewName, features }: FeaturesListProps) => { | ||
const { enabledFeatureStatistics } = useContext(FeatureFlagsContext); |
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.
is this intentional? seems not used so maybe can revert
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 is intentional. This enables the user to turn on this on as a prop on <FeastUI>
i.e.
<FeastUI
feastUIConfigs={{
featureFlags: {
enabledFeatureStatistics: true
},
}}
/>
This already works. I need to document this better hahaha.
@@ -0,0 +1,10 @@ | |||
import React from "react"; | |||
|
|||
interface FeatureFlags { |
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.
does this let users flip it on / off in the top level FeastUI component?
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.
Yup
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.
See this comment: #2370 (comment)
ede4141
to
887c892
Compare
Is this really a "feature"? Seems more like a chore or housekeeping. Bit of a gray area. |
I didn't know what to pick. Happy to go with whatever @adchia thinks is more appropriate. |
Couple things to fix:
|
Also sign the commits |
Also: revert the deleting of the App.test.tsx |
convert it from feat: -> housekeeping: |
Any reason we can't do the following instead
|
@adchia @tonyhschu next steps here? |
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
Signed-off-by: Tony Chu <tonyhschu@gmail.com>
887c892
to
a58c59c
Compare
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adchia, tonyhschu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Makes the Feast UI importable as a NPM Module. This will make it easier for customers that want to extend Feast UI to have maintain their own repo while still consuming our changes.
Which issue(s) this PR fixes:
Fixes #