tracking attribute path into options[:attr_path] #139
Conversation
|
I really like this, needs a CHANGELOG entry, please. |
| begin | ||
| next unless should_return_attribute?(attribute, opts) && conditions_met?(exposure_options, opts) | ||
|
|
||
| partial_output = value_for(attribute, opts) |
There was a problem hiding this comment.
Maybe construction of partial_output can be refactored into a method?
some refines; new test cases; change log updated
calfzhou
force-pushed
the
feature/attr-path
branch
from
c8c8194
to
fa05156
Compare
|
squashed and rebased to current master. |
tracking attribute path into options[:attr_path]
|
Merged. |
| @@ -7,12 +7,12 @@ | |||
|
|
|||
| # Offense count: 8 | |||
| Metrics/AbcSize: | |||
| Max: 51 | |||
| Max: 53 | |||
There was a problem hiding this comment.
Metrics should not increase. Why we use RuboCop if we just stab it?
There was a problem hiding this comment.
Personally I don't care as much, I think it's a good flag and if it makes sense, I would not make it a barrier.
There was a problem hiding this comment.
Yes I think the same but I just want to see things before changing .rubocop_todo.yml. There could be something we can suggest to easily fix and if there is not then we suggest to just regenerate .rubocop_todo.yml :)
There was a problem hiding this comment.
As features and their correctness are superior to style pedantry, I want to push #160.
I think the reason that people just stab RuboCop offenses is that it runs before RSpec. It prevents maintainers to just see if the feature implemented correctly or not.
There was a problem hiding this comment.
I would like to disable this offense in Rubocop, and be able to run --auto-gen-config for everything else. However when you try to do the latter it ignores what's in .rubocop.yml, which defeats the whole purpose.
Feel free to switch it to run after, btw.
|
I ported this feature while working on rebasing #151. But I changed your specs because in the new design there's no place for methods like Instead of this I implemented an expose :charactersistics, attr_path: -> (obj, opts) { :character }
expose :need_to_skip, attr_path: -> (obj, opts) { nil }Maybe it would be useful to also add these possibilities in future: expose :need_to_skip, attr_path: :character # just constant
expose :need_to_skip, attr_path: nil # just skip |
|
Thanks @marshall-lee your changes make sense to me. |
|
@marshall-lee 's |
Make Grape::Entity keep tracking of current being exposed attribute's full path into
options[:attr_path].