Closed jorritfolmer closed 6 years ago
@jorritfolmer Thanks for the PR, it is possible to add tests for the new feature, I just want to make sure we don't break it a future patch.
@ph yes I can add tests to see that the issues we're having with the Netflow codec are in fact resolved (and remain resolved). Like https://github.com/jorritfolmer/logstash-input-netflow/blob/master/spec/inputs/netflow_spec.rb#L1367-L1387.
The tests will include the Netflow codec and its output, so they'll be more integration tests than unit-tests if that's ok.
@jorritfolmer Thats OK to me! lets create a new directory named spec/integration/
and put the new spec there.
I think someone is working on a netflow input to replace the netflow codec, btw. Does this impact how we treat this PR?
On Thursday, August 18, 2016, Jorrit Folmer notifications@github.com wrote:
Because /logstash-plugins/logstash-codec-netflow/issues/9
You can view, comment on, or merge this pull request online at:
https://github.com/logstash-plugins/logstash-input-udp/pull/25 Commit Summary
- Add identity_map_codec support
File Changes
- M lib/logstash/inputs/udp.rb https://github.com/logstash-plugins/logstash-input-udp/pull/25/files#diff-0 (8)
Patch Links:
- https://github.com/logstash-plugins/logstash-input-udp/pull/25.patch
- https://github.com/logstash-plugins/logstash-input-udp/pull/25.diff
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/logstash-plugins/logstash-input-udp/pull/25, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIC6u-m5nFMNEfzCmKhN7uUi6MU6Airks5qhTFigaJpZM4JoIyO .
Yes netflow input would be me, too. You choose :)
Because https://github.com//logstash-plugins/logstash-codec-netflow/issues/9