fix: RuboCop #371
fix: RuboCop #371
Conversation
troycoll
left a comment
troycoll
left a comment
There was a problem hiding this comment.
No AI agents were harmed in the reviewing of this PR.
| metrics.map { |k, v| ["#{type}##{k}", v] }.to_h | ||
| end | ||
|
|
||
| private_class_method :add_prefix |
There was a problem hiding this comment.
This is technically a breaking change.
There was a problem hiding this comment.
Yes, thats correct. My plan is to cut Pliny 2. I want to make some other breaking changes as well. PLINY_ENV is deprecated in favor of APP_ENV and I would like to remove support for PLINY_ENV.
| -> (v) do | ||
| if v | ||
| v.split(",").map{|a| cast(a, method) } | ||
| end | ||
| ->(v) do | ||
| v&.split(",")&.map { |a| cast(a, method) } |
There was a problem hiding this comment.
I think this one is curious because it's not exactly equivalent. Is it a safe correction?
We know that String#split will always return an array but RuboCop doesn't because we don't have any type definitions. Before, it would cause an error if it suddenly returned nil and now it will instead return that.
There was a problem hiding this comment.
Thats true. The only way I can think of this happening if we have a custom split() method on what ever we are passing to the config block. The context here is mapping ENV vars to methods and casting the input (ENV var) to primitives (int, string, symbol) I think it should not be possible to get nil from split()
There was a problem hiding this comment.
I'm not saying there's anything wrong with the correction in our concrete case. I was just curious if RuboCop thought it was equivalent.
There was a problem hiding this comment.
The RuboCop rules don't go that deep. It just sees the first safe navigator and then assumes that all following method calls need it as well. It's complaining about Lint/SafeNavigationChain
It's a combination of 2 rules. first it sees the if v and replaces that with v& and after that the Lint/SafeNavigationChain kicks in
Co-authored-by: Troels Thomsen <19824+tt@users.noreply.github.com>
fix identified RuboCop issues