Test pull request – checks if Hound works fine with Thoughtbot's config #56
Conversation
| @@ -52,7 +52,7 @@ def progressbar_for_model(klass) | |||
| title: klass.name, | |||
| total: klass.unscoped.count, | |||
| throttle_rate: 0.1, | |||
| format: %q[%t %c/%C (%j%%) %B %E], | |||
| format: %q[%t %c/%C (%j%%) %B %E] | |||
There was a problem hiding this comment.
Put a comma after the last parameter of a multiline method call.
Use %q only for strings that contain both single quotes and double quotes.
lib/attr_masker/attribute.rb
Outdated
| @@ -34,7 +34,7 @@ def should_mask?(model_instance) | |||
| # returning. | |||
| def mask(model_instance) | |||
| value = unmarshal_data(model_instance.send(name)) | |||
| masker_value = options[:masker].call(options.merge!(value: value)) | |||
| masker_value = options[:masker].call(options.merge!({value: value})) | |||
There was a problem hiding this comment.
Space inside { missing.
Redundant curly braces around a hash parameter.
Space inside } missing.
Codecov Report
@@ Coverage Diff @@
## master #56 +/- ##
=======================================
Coverage 99.39% 99.39%
=======================================
Files 23 23
Lines 333 333
=======================================
Hits 331 331
Misses 2 2
Continue to review full report at Codecov.
|
lib/attr_masker/performer.rb
Outdated
| @@ -11,7 +11,7 @@ def mask | |||
| # | |||
| if Rails.env.production? | |||
| unless ENV['FORCE_MASK'] | |||
| raise AttrMasker::Error, "Attempted to run in production environment." | |||
| raise AttrMasker::Error, 'Attempted to run in production environment.' | |||
lib/attr_masker/performer.rb
Outdated
| @@ -11,7 +11,8 @@ def mask | |||
| # | |||
| if Rails.env.production? | |||
| unless ENV['FORCE_MASK'] | |||
| raise AttrMasker::Error, "Attempted to run in production environment." | |||
| msg = 'Attempted to run in production environment.' | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
It looks like a very good renovation. |
|
@skalee can we merge this? Thanks! |
There was a problem hiding this comment.
Some files could not be reviewed due to errors:
.rubocop.yml: Lint/ConditionPosition has the wrong namespace - should be Layout
.rubocop.yml: Lint/ConditionPosition has the wrong namespace - should be Layout Error: The `Style/TrailingCommaInLiteral` cop no longer exists. Please use `Style/TrailingCommaInArrayLiteral` and/or `Style/TrailingCommaInHashLiteral` instead. (obsolete configuration found in .rubocop.yml, please update it)
|
Well… this pull request was never intended to be merged into master. I've created it only to test if Hound CI works correctly with our improved style guide. We were doing some bigger structural changes back then (see riboseinc/oss-guides#3), and I needed some place to experiment. Apparently I forgot to close the pull request afterwards. These experiments were quite successful, and our current style guide is highly inspired with them. You can find the current version of our style guide here: https://github.com/riboseinc/oss-guides. Moreover, everything worth merging (IMHO) is already on master since merging #57. I think that only leftovers are here, and I am going to close this pull request unless someone opposes. That said, there is one thing to note. The style guide in this project wasn't updated for a while, and when you wrote your comments, it wasn't compatible with Rubocop 0.54. But it's a trivial change, and I've already took care of that in #61. |
|
Thank you @skalee ! Let’s mark the experiment a success and close it. |
No description provided.