Specify in Readme that global rubocop command is not supported #1911
Conversation
vscode/README.md
Outdated
|
|
||
| ```ruby | ||
| # Gemfile | ||
| group :development do |
There was a problem hiding this comment.
Linters usually need to also run in CI, so let's say group :development, :test do. This matches what Rails 8 will do: https://github.com/rails/rails/blob/main/railties/lib/rails/generators/rails/app/templates/Gemfile.tt
vscode/README.md
Outdated
| ```ruby | ||
| # Gemfile | ||
| group :development do | ||
| # You can uncomment one of the following to get either rubocp or syntax_tree formatter enabled. |
There was a problem hiding this comment.
Since this is an example, I think it's enough to just say gem "rubocop" and remove the comment.
vscode/README.md
Outdated
| ``` | ||
|
|
||
| Make sure to run `bundle install` after making the above change. | ||
|
|
There was a problem hiding this comment.
Is this the final new line or an extra empty line at the end? If it's an empty line, can we please remove it?
|
@bibstha since these instructions aren't specific to VS Code, we should add them to the main README instead. I'd suggest adding a new section, e.g. 'Formatting and Linting'. Once added, we can add a link to it from the VS Code README to help people find it. |
|
Hi @bibstha, are you able to continue on this? After discussing with @vinistock, we think it could be better to have an explicit statement like “Using globally installed formatters or linters is not supported, they must in your Gemfile or gemspec”. Then there is not any need for the sample code. |
|
Yes, will make the change later today. |
|
@andyw8 made the change. |
bibstha
changed the title
vinistock
added
blocked
Motivation
Since I was instructed to open a PR in this repo in Shopify/vscode-shopify-ruby#595, doing so now. cc: @vinistock
Shopify/vscode-shopify-ruby#443
It's easier to glance over the line that says to add
rubocoporsyntax_treegem in Gemfile. Adding a concrete code example makes it find the solution.I've seen few people ask about this in support issues in the repo as well.
Implementation
Just the readme update.
Automated Tests
NA
Manual Tests
NA