Update library syntax #129
Conversation
alextwoods
left a comment
alextwoods
left a comment
There was a problem hiding this comment.
Nice - looking good! So many double quotes....
.rubocop.yml
Outdated
| Enabled: false | ||
|
|
||
| Metrics/LineLength: | ||
| Enabled: false |
There was a problem hiding this comment.
Our other projects have traditionally set this at 80 (though personally I prefer 120). If we set it to 80 or 120, how many violations does that add?
There was a problem hiding this comment.
I like 80 because I can have three windows on my monitor and not have horizontal scrolling. I'm ok with 120 if it's overly burdensome, but I think most of the library is already 80
There was a problem hiding this comment.
Thanks y'all for the inputs. I went with 120 max, since I was encountering 150+ more offenses with 80 max.
lib/aws-record/record/batch_read.rb
Outdated
|
|
||
| @items.each { |item| yield item } | ||
| @items.each do |item| | ||
| yield item |
There was a problem hiding this comment.
slightly inconsistent style between yield(batch) and yield item. I think I'd lean towards calling with the parens here.
There was a problem hiding this comment.
Updated for consistency. TY!
.rubocop.yml
Outdated
| Enabled: false | ||
|
|
||
| Metrics/MethodLength: | ||
| Max: 50 |
There was a problem hiding this comment.
This feels a bit arbitrary here - I think I'd actually lean towards just disabling.
There was a problem hiding this comment.
For one off cases, you can add inline comment to disable cops
There was a problem hiding this comment.
I went with disabling altogether as there quite a few...! Thanks!
.rubocop.yml
Outdated
| - 'spec/**/*' | ||
|
|
||
| Style/SymbolArray: | ||
| EnforcedStyle: brackets |
There was a problem hiding this comment.
Why these styles? I think as much as possible I'd lean towards rubocop defaults. Or does this syntax break something else?
There was a problem hiding this comment.
I also prefer defaults (can't thumbs up from mobile)
alextwoods
left a comment
alextwoods
left a comment
There was a problem hiding this comment.
LGTM - Great cleanups!
3 participants
Description of changes:
Changes made to the library to be up to date with Ruby Style Guide with a help of rubocop.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.