-
Notifications
You must be signed in to change notification settings - Fork 452
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
Resolve new chefstyle warnings & use safe navigators #1507
Conversation
Signed-off-by: Tim Smith <tsmith@chef.io>
lib/ohai/plugins/linux/network.rb
Outdated
r.merge(dev: i) if r[:family] == family[:name] | ||
end.compact | ||
end | ||
end&.compact |
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 it'd be more readable to add a next unless iv[:routes]
or similar at the top of the loop instead.
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 that's probably a good case where the alternative is most likely way more readable.
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'm not a huge fan of the pretzel operator but I could just be being a curmudgeon 😅
We can avoid some duplicate checks by using &. in places Signed-off-by: Tim Smith <tsmith@chef.io>
(I kinda like the pretzel-dot operator, but some of these use cases are a bit funny) |
There's no need for the &. here Signed-off-by: Tim Smith <tsmith@chef.io>
Also add some notes here that we should convert this to a filter_map when we can as that's easier to read and faster. Perf benchmarks from the tubes: Signed-off-by: Tim Smith <tsmith@chef.io>
Nuked the weird ones. We should be good now |
We can avoid some duplicate checks by using &. in places