-
Notifications
You must be signed in to change notification settings - Fork 218
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
refactor(ui): move Segments to use Redux storage #2441
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2441 +/- ##
=======================================
Coverage 70.76% 70.76%
=======================================
Files 81 81
Lines 8110 8110
=======================================
Hits 5739 5739
Misses 2025 2025
Partials 346 346 ☔ View full report in Codecov by Sentry. |
authorized response. make eslint more happy
@@ -61,12 +61,10 @@ export default function Rules() { | |||
const namespace = useSelector(selectCurrentNamespace); | |||
const readOnly = useSelector(selectReadonly); | |||
|
|||
const segments = useListSegmentsQuery(namespace.key)?.data?.segments || []; | |||
|
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.
@erka is this something to worry about? ☝🏻
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.
I modified the code to address it.
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.
one outstanding question, otherwise lgtm! I tested with setting up GitHub auth too to ensure the headers were passed correctly 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.
Looks great. Thank you @erka 😊
Theres currently something broken with our fs/git integration tests on main, looking into it now |
Try to use Redux RTK for Segments.
listSegments
Refs: #2433