Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify in Readme that global rubocop command is not supported #1911

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

bibstha
Copy link
Contributor

@bibstha bibstha commented Apr 10, 2024

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 rubocop or syntax_tree gem 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

@bibstha bibstha requested a review from a team as a code owner April 10, 2024 17:54
vscode/README.md Outdated Show resolved Hide resolved
vscode/README.md Outdated

```ruby
# Gemfile
group :development do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is an example, I think it's enough to just say gem "rubocop" and remove the comment.

@bibstha bibstha requested a review from andyw8 April 10, 2024 18:51
vscode/README.md Outdated
```

Make sure to run `bundle install` after making the above change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@egiurleo egiurleo removed their request for review April 10, 2024 21:46
@andyw8
Copy link
Contributor

andyw8 commented Apr 11, 2024

@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.

@KaanOzkan KaanOzkan removed their request for review April 12, 2024 13:31
@andyw8
Copy link
Contributor

andyw8 commented May 10, 2024

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.

@bibstha
Copy link
Contributor Author

bibstha commented May 10, 2024

Yes, will make the change later today.

@bibstha bibstha requested a review from a team as a code owner May 21, 2024 18:55
@bibstha bibstha requested review from st0012 and vinistock May 21, 2024 18:55
@bibstha
Copy link
Contributor Author

bibstha commented May 21, 2024

@andyw8 made the change.

@bibstha bibstha changed the title Add code example of how one can enable ruby formatter in VS Code Specify in Readme that global rubocop command is not supported May 21, 2024
vscode/README.md Outdated Show resolved Hide resolved
@bibstha bibstha requested a review from vinistock May 29, 2024 15:43
@vinistock vinistock added the documentation Improvements or additions to documentation label May 29, 2024
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but style checks are failing. You can fix them automatically by:

  1. cd vscode
  2. yarn install
  3. yarn run format

@bibstha bibstha requested a review from vinistock June 7, 2024 19:30
@vinistock vinistock added blocked This issue can't move forward until a blocker is resolved and removed blocked This issue can't move forward until a blocker is resolved labels Jun 10, 2024
@vinistock vinistock merged commit bed467e into Shopify:main Jun 10, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants