Fix v6 ipfix export #94
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #94 +/- ##
==========================================
- Coverage 41.54% 41.42% -0.13%
==========================================
Files 29 29
Lines 1993 1999 +6
==========================================
Hits 828 828
- Misses 1127 1133 +6
Partials 38 38
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
jotak
added
no-doc
|
/lgtm |
| err = dataSet.AddRecord(ipf.entitiesV4, templateID) | ||
| if err != nil { | ||
| return err | ||
| if v6 { |
There was a problem hiding this comment.
its good practice to avoid if() {
} else {
}
instead
err = dataSet.AddRecord(ipf.entitiesV4, templateID)
if err != nil {
return err
}
if v6 {
err = dataSet.AddRecord(ipf.entitiesV6, templateID)
if err != nil {
return err
}
}There was a problem hiding this comment.
@msherif1234 Wouldn't that perform dataSet.AddRecord multiple times for v6?
There was a problem hiding this comment.
+1 @praveingk
I think the good practice referred to is writing if { ...return } ... return instead of if { ...return } else { ...return}, but it doesn't apply here as not all paths return in the conditional expressions
There was a problem hiding this comment.
yeah @praveingk I was commenting in general and yes whatu have make sense
/lgtm
There was a problem hiding this comment.
Hi @praveingk
so we can do this
entities := ipf.entitiesV4
if v6 {
entities = ipf.entitiesV6
}
if err := dataSet.AddRecord(entities, templateID); err != nil {
return err
}
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak 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 |
This PR fixes #93