-
Notifications
You must be signed in to change notification settings - Fork 1
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: Update metric length limits #46
Conversation
@@ -122,43 +120,3 @@ describe('useComponentMetrics', () => { | |||
); | |||
}); | |||
}); | |||
|
|||
describe('PanoramaClient', () => { |
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.
Extracted into a separate file, because this code does not have much to do with useComponentMetrics
hook
console.error(`Event context for metric is too long: ${metric.eventContext}`); | ||
return; | ||
} | ||
if (!validateLength(metric.eventType, 50)) { |
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.
Added validation for all supported keys. This one is especially important, because it is shorter than others and we are likely to hit this limit
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
==========================================
+ Coverage 94.52% 98.36% +3.84%
==========================================
Files 16 16
Lines 292 306 +14
Branches 57 62 +5
==========================================
+ Hits 276 301 +25
+ Misses 16 5 -11 ☔ View full report in Codecov by Sentry. |
0127019
to
1afaf8b
Compare
1afaf8b
to
34c8546
Compare
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import { MetricsTestHelper, Metrics } from '../metrics/metrics'; |
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.
Tests moved from components repo: cloudscape-design/components#1919
Issue #, if available:
Description of changes:
Bring these values up to date with the receiver's end.
"Dry-run / dry-run / Components unit tests" is expected to fail. They will pass after merging cloudscape-design/components#1919All passes nowBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.