Skip to content
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

NETOBSERV-970 Typo in Query Summary panel #368

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented Aug 9, 2023

Here is my suggestion to fix the typo:

  • Non standard units are written entirely (Flows, Packets)
  • Factor is placed before the limit reached indicator for these (1k+ Flows, 1k+ Packets) and kept after for others (757+ MB, 2.52+ MBps)

image

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #368 (aca1f9c) into main (99ba7d1) will decrease coverage by 0.48%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #368      +/-   ##
==========================================
- Coverage   57.77%   57.29%   -0.48%     
==========================================
  Files         166      167       +1     
  Lines        7659     7737      +78     
  Branches      915      917       +2     
==========================================
+ Hits         4425     4433       +8     
- Misses       2967     3038      +71     
+ Partials      267      266       -1     
Flag Coverage Δ
uitests 58.16% <100.00%> (+0.06%) ⬆️
unittests 54.91% <ø> (-1.96%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...c/components/query-summary/flows-query-summary.tsx 96.00% <ø> (ø)
web/src/components/__tests-data__/flows.ts 100.00% <100.00%> (ø)
web/src/utils/format.ts 100.00% <100.00%> (+16.66%) ⬆️

... and 1 file with indirect coverage changes

@@ -453,6 +452,7 @@
"Specify a single DNS RCODE name like:": "Specify a single DNS RCODE name like:",
"A IANA RCODE number like 0, 3, 9": "A IANA RCODE number like 0, 3, 9",
"A IANA RCODE name like NoError, NXDomain, NotAuth": "A IANA RCODE name like NoError, NXDomain, NotAuth",
"P": "P",
Copy link
Contributor

@msherif1234 msherif1234 Aug 10, 2023

Choose a reason for hiding this comment

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

what is P ? Packet(s) ? if so u changed it below from P to Packets then we probably need to do the same here too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an autogenerated file 😃

However i18n is a bit dumb and sort translations by first found in sources.
'P' is still used in Overview page since we don't have room to write 'Packets' inside donuts so that's why it's still there but in another order

Copy link
Contributor

Choose a reason for hiding this comment

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

/lgtm

@Amoghrd
Copy link
Contributor

Amoghrd commented Aug 10, 2023

/ok-to-test

1 similar comment
@msherif1234
Copy link
Contributor

/ok-to-test

@msherif1234 msherif1234 added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 10, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/network-observability-console-plugin:d15182a

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=d15182a make set-plugin-image

@Amoghrd
Copy link
Contributor

Amoghrd commented Aug 10, 2023

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Aug 10, 2023
@jpinsonneau
Copy link
Contributor Author

/approve

@openshift-ci
Copy link

openshift-ci bot commented Aug 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpinsonneau

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit dce0ee5 into netobserv:main Aug 11, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. qe-approved QE has approved this pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants