-
Notifications
You must be signed in to change notification settings - Fork 26
Add to_string and tests for Ip Addresses so it returns the ip address with the cidr (ex: 10.0.0.0/24) #99
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
base: master
Are you sure you want to change the base?
Conversation
lib/netbox_client_ruby/entities.rb
Outdated
| else | ||
| if netbox_object.respond_to?(filter_key) | ||
| netbox_object.public_send(filter_key).to_s == filter_value.to_s | ||
| if netbox_object.public_send(filter_key).respond_to?(:to_string) |
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.
I wonder if there is a better solution for this issue 🤔 . This would apply for all resources and it's not very clear if I just look at the code why this happens.
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.
I would've added a comment into the code, but I was trying to stick to your style.
I can do that if you want.
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.
I think more about checking for IPAddress explicitly and use to_string there instead of to_s.
case netbox_object
when IPAddress
...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.
Yeah, I was thinking that too. I just haven't updated the PR yet.
The only downside is if you decide to swap IpAddress for something like ipaddr, which also uses to_string.
|
FIY, I submitted a RP to the IpAddress gem to update their README. Man, I spent more time than I'd like to admit, trying to figure out where I went wrong, before realizing their README was wrong. |
Closes #98