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

Only require RuboCop diagnostic inside the formatter #2610

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

vinistock
Copy link
Member

@vinistock vinistock commented Sep 24, 2024

Motivation

Closes #2604

The RuboCop diagnostic class should only be required if RuboCop is present. In #2589, I mistakenly moved the require to internal.rb, but trying to require it when RuboCop is not present fails.

Implementation

Moved the require inside the class that needs the diagnostic class. That require will only occur if RuboCop is present.

Manual tests

  1. Remove all rubocop related gems and syntax tree from the Gemfile
  2. Reload VS Code
  3. Verify it doesn't crash

@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Sep 24, 2024
@vinistock vinistock self-assigned this Sep 24, 2024
@vinistock vinistock requested a review from a team as a code owner September 24, 2024 13:37
@vinistock vinistock requested review from andyw8 and st0012 September 24, 2024 13:37
@andyw8
Copy link
Contributor

andyw8 commented Sep 24, 2024

Let's ship this but I like @Earlopain's suggestion in the other PR of having a CI job that doesn't have RuboCop available.

@vinistock
Copy link
Member Author

Fair. We should totally add that, but I do want to cut the release with the fix asap.

@vinistock vinistock enabled auto-merge (squash) September 24, 2024 13:43
@vinistock vinistock merged commit 0ef3713 into main Sep 24, 2024
37 checks passed
@vinistock vinistock deleted the vs-only-require-diagnostic-inside-formatter branch September 24, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hidden dependency on rubocop in ruby-lsp 0.18.2
2 participants