Change our internal RuboCop integration identifier #3067
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
vinistock
added
bugfix
Graphite App
3 tasks
vinistock
force-pushed
the
01-15-change_our_internal_rubocop_integration_identifier
branch
from
8b194c7 to
b6f67d1
Compare
vinistock
force-pushed
the
01-15-change_our_internal_rubocop_integration_identifier
branch
2 times, most recently
from
9793189 to
fe108cd
Compare
andyw8
reviewed
Jan 16, 2025
andyw8
approved these changes
Jan 16, 2025
vinistock
force-pushed
the
01-15-change_our_internal_rubocop_integration_identifier
branch
from
fe108cd to
798e8a1
Compare
Merge activity
|
vinistock
deleted the
01-15-change_our_internal_rubocop_integration_identifier
branch
vinistock
added a commit
that referenced
this pull request
Jan 16, 2025
### Motivation I wanted to upgrade RuboCop to the latest version so that we can verify #3067 is working. After upgrading, I noticed that Tapioca was broken because of our version of Bundler, which is fixed in a recent release - so I upgraded that too. Then I updated our gem RBIs and fixed any issues. I split commit by commit to make it easier to review.
vinistock
added a commit
that referenced
this pull request
Jan 16, 2025
### Motivation This is a fix for the bug I just introduced in #3067 🤦♂️. We cannot assume that the `RuboCop` constant will be defined because projects may use a different formatter. We need to check first. ### Implementation Started checking if the constant we need is defined as part of the check. ### Automated Tests Part of why this wasn't caught is because our integration tests were ignoring the response return for the initialize request. That makes the tests extremely weak because the LSP will actually rescue anything that happens during initialize and then simply return an error back to the client, which means that tests passed despite initialization failing. I started properly checking for what response was returned, so that we can fail tests if initialization was not successful. Without the fix, tests now properly fail.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
This PR addresses point 1 and 2 from #3046
Now that the RuboCop add-on is using the
rubocopidentifier, we need to change ours to prevent conflicts, while still ensuring that users who are on older versions of RuboCop are getting the right behaviour.Implementation
Essentially, we are changing our identifier to
rubocop_internal.The only extra logic is falling back when users have explicitly configured their formatter or linter as
rubocop. Since that is now owned by RuboCop, but only present on versions higher than v1.70, we need to check the version and ensure that it's actually available or fallback to our internal integration.Automated Tests
Added tests.