Conversation
kaisecheng
left a comment
kaisecheng
left a comment
There was a problem hiding this comment.
Hi @ldormoy , sorry for the delay and thanks for submitting the PR! The doc and test look good. It works as it supposes to.
I would like to address the duplication before merging it.
Could you also update the version in https://github.com/logstash-plugins/logstash-filter-fingerprint/blob/master/logstash-filter-fingerprint.gemspec#L4 and https://github.com/logstash-plugins/logstash-filter-fingerprint/blob/master/CHANGELOG.md ?
lib/logstash/filters/fingerprint.rb
Outdated
| ) | ||
| end | ||
| class << self; alias_method :fingerprint, :fingerprint_ipv4_network; end | ||
| when :IPV6_NETWORK |
There was a problem hiding this comment.
As the implementation is the same as IPv4, I suggest to reuse the condition in ipv4 instead of duplicate the code
when :IPV4_NETWORK, :IPV6_NETWORK
lib/logstash/filters/fingerprint.rb
Outdated
| IPAddr.new(ip_string).mask(@key.to_i).to_s.force_encoding(Encoding::UTF_8) | ||
| end | ||
|
|
||
| def fingerprint_ipv6_network(ip_string) |
There was a problem hiding this comment.
same as above, we can remove the duplication
kaisecheng
left a comment
kaisecheng
left a comment
There was a problem hiding this comment.
As mentioned, once it addresses, I can merge it
|
❌ Author of the following commits did not sign a Contributor Agreement: Please, read and sign the above mentioned agreement if you want to contribute to this project |
|
@ldormoy can you sign the contributor agreement to unblock me to merge? |
4 participants
Solves #49
This PR adds an IPV6_NETWORK method, working the same way as IPV4_NETWORK.