-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[refractor] Switch to enums for ES mappings #6091
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
[refractor] Switch to enums for ES mappings #6091
Conversation
Signed-off-by: Saumya Shah <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6091 +/- ##
==========================================
- Coverage 96.93% 96.92% -0.01%
==========================================
Files 352 352
Lines 16719 16742 +23
==========================================
+ Hits 16206 16227 +21
- Misses 329 330 +1
- Partials 184 185 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Can we improve test coverage for the new code? |
Should we add tests for |
The codecov comment #6091 (comment) shows all missing coverage. Is there any reason not to test all gaps? |
Signed-off-by: Saumya Shah <[email protected]>
We were missing out on testing |
Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
…umya40-codes/jaeger into switch-to-enums-for-esmapping
Head branch was pushed to by a user without write access
Signed-off-by: Saumya Shah <[email protected]>
Anything else I'm missing on @yurishkuro ? |
Thanks! |
## Which problem is this PR solving? - Resolves jaegertracing#6067 - Follows up jaegertracing#6068 ## Description of the changes - Currently in `plugin/storage/es/mappings/mapping.go` we are using bare string inorder to for the file mapping. This PR refractors this approach to use enums instead of string ## How was this change tested? - running `go test` in respective files/directories ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Saumya Shah <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
…)" This reverts commit a9f9fcb.
Which problem is this PR solving?
Description of the changes
plugin/storage/es/mappings/mapping.go
we are using bare string inorder to for the file mapping. This PR refractors this approach to use enums instead of stringHow was this change tested?
go test
in respective files/directoriesChecklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test