-
Notifications
You must be signed in to change notification settings - Fork 785
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
Added ActivityEvent.EnumerateTags & ActivityLink.EnumerateTags extensions #1320
Added ActivityEvent.EnumerateTags & ActivityLink.EnumerateTags extensions #1320
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1320 +/- ##
==========================================
+ Coverage 83.81% 83.88% +0.07%
==========================================
Files 347 347
Lines 13362 13412 +50
==========================================
+ Hits 11199 11251 +52
+ Misses 2163 2161 -2
|
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.
It's a nice improvement!
nit: we should somehow try to merge all this functions to one, since all looks the same...not sure if that's feasible
@eddynaka I combined ActivityTagObjectsEnumeratorFactory & ActivityTagsCollectionEnumeratorFactory into ActivityTagsEnumeratorFactory. So now there are 3 factories: 1 for tags (activity.TagObjects, activityLink.Tags, & activityEvent.Tags), 1 for events (activity.Events), & 1 for links (activity.Links). How's that? |
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.
LGTM
Added extension methods for enumerating tags on Activity Events & Links without allocation. Updated Jaeger. Removed some code from the EnumerationHelper that is no longer needed. Renamed Activity.EnumerateTagValues -> Activity.EnumerateTags.
TODOs:
CHANGELOG.md
updated for non-trivial changes