NETOBSERV-1580 Reuse frontend config #105
Conversation
|
Skipping CI for Draft Pull Request. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
- Coverage 24.24% 22.66% -1.58%
==========================================
Files 9 10 +1
Lines 1415 1337 -78
==========================================
- Hits 343 303 -40
+ Misses 1051 1015 -36
+ Partials 21 19 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -397,6 +345,30 @@ func updateTable() { | |||
| } | |||
| } | |||
|
|
|||
| func cycleOption(selection []string, exclusiveOptions []string, options []string, incr int) []string { | |||
There was a problem hiding this comment.
could you create a unit test for this one? I find it hard to understand, so some examples-by-test would help :-)
There was a problem hiding this comment.
also maybe a comment such as (if I understand correctly):
Cycles through options, with exclusive options first, then non-exclusive options, and last the full list of non-exclusive options all at once.
There was a problem hiding this comment.
@jpinsonneau did you see my comments here ?
I forgot you were doing this and ended up something similar here https://github.com/netobserv/network-observability-cli/pull/137/files#diff-a86c8e951a1155a451b2d254dfecbb2d151455820c7954bc9795a96c8379b2a5R16-R24 , which I'll remove this you were here first :-)
But I have the feeling it can be simplified when using more structured data... at the end of the day, cycling is just incrementing or decrementing an index that could be persisted in memory rather than having to re-compute it by lookups and matching
There was a problem hiding this comment.
Created https://issues.redhat.com/browse/NETOBSERV-2016 to address that in a followup 😉
Thanks !
|
Config changes + rebase broke the UI: |
|
/lgtm But I feel like this would be simpler ? I'll double-check based on this PR later, if that would still work with it, maybe it won't |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: 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 |
Description
FYI columns naming changed since hardcoded values were not matching current configuration.
Dependencies
Based on #92
Operator config update: netobserv/network-observability-operator#819
Console plugin config update: netobserv/network-observability-console-plugin#621
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.