Add rubocop, fix syntax #32
Conversation
| require "rspec/core/rake_task" | ||
| require 'bundler/gem_tasks' | ||
| require 'rspec/core/rake_task' | ||
| require 'rubocop/rake_task' |
There was a problem hiding this comment.
Can we make the rule: always double quotes?
There was a problem hiding this comment.
Reasoning for not using single quotes? Rubocop does not have a rule for forcing double quotes on non-interpolationed strings
There was a problem hiding this comment.
- For consistency; doing things one way, is better than doing things two ways. I'd like to stick with one form of quotes unless the situation requires something else. Double quotes fits most situations so we should stick with that.
- For development speed; I don't want to have to change the quoting symbol when adding or removing interpolation to a string. That wastes time and effort.
- Recommended in GitHub's styleguide for ruby https://github.com/styleguide/ruby
There was a problem hiding this comment.
Hmm i wouldn't agree with the consistency part (Example: https://github.com/envato/pagerduty/pull/32/files#diff-b2dabed64552f7bd74f43e99df8d27dbR78). Most project use ' instead of " per option A of the original style guide at https://github.com/bbatsov/ruby-style-guide#consistent-string-literals
Robocop does not have a cop to force all double so there will be no checks on consistency if it switch it back.
Let me know if you still think it should be switched.
There was a problem hiding this comment.
The instance you point to has a good reason to not use double quote delimiters; the string contains double quotes. To use double quote delimiters would mean escaping all the " characters. I don't advocate a hard rule of using " all the time. Rather I find it fits most of the time. So it's a good default. I like pragmatism.
Note that the style guide says:
The second style is arguably a bit more popular in the Ruby community. The string literals in this guide, however, are aligned with the first style.
That means " is arguably more popular than '.
There was a problem hiding this comment.
Right, however this repo isn't standardized on ".
shortdudey123
force-pushed
the
beef_up_testing
branch
from
c974690
to
96ce2b0
Compare
| @@ -19,7 +18,7 @@ | |||
| expect(transport).to have_received(:send_payload).with( | |||
| service_key: "a-test-service-key", | |||
| event_type: "trigger", | |||
| description: "a-test-description", | |||
| description: "a-test-description" | |||
There was a problem hiding this comment.
Does Rubocop check for trailing ,? What's the rationale behind this one?
I find it quite handy to have trailing ,; it's then fast to add lines and the diff is cleaner.
There was a problem hiding this comment.
|
Thanks @shortdudey123! |
3rd PR from breaking up #27
I can break up each major syntax change into a separate commit if needed (i.e. quotes, 1.9 hash style, etc...)