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

Fix backward compatibility for Bundler versions < 2.2.17 #3152

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Artes02
Copy link

@Artes02 Artes02 commented Feb 8, 2025

Motivation

Hi!

One of the projects that I'm working with still uses Bundler version 2.1.4 (sad, I know!) and Ruby LSP errors out upon initialization with this version. In fact, it won't be able to initialize with any Bundler prior to version 2.2.17 (more details in Implementation section)!

I believe that Ruby LSP is now an essential part of any VS Code's Ruby project and I want to make it backward compatible with previous versions of Bundler so that people don't have to switch to other editors/IDEs when they can't simply upgrade to newer version of Bundler due to certain project limitations.

Luckily, the fix is not that complicated!

Implementation

Let's take a look at the PR that introduced the usage of Bundler::Settings.key_for(e) in Ruby LSP: #2535.

It might look okay at first until you realize that def self.key_for(key) for Bundler::Settings was only introduced in bundler-v2.2.17, meaning that Bundler::Settings.key_for(e) won't work for any Bundler version < 2.2.17.

Considering that instance method def key_for(key) is still in place in latest Bundler version and don't seem to be going away - I suggest we use it instead! Plus we already have an instance of Bundler::Settings available in our local variable settings.

I was also thinking about an explicit Bundler version check approach but didn't find any benefits. There's even a drawback - we'd be missing out the caching mechanism (introduced by this PR) that instance method provides in newer versions of Bundler if we keep using the class method instead.

Automated Tests

I updated corresponding test/setup_bundler_test.rb.

Manual Tests

Actual result:

GIVEN ruby project with Bundler version < 2.2.17
WHEN Ruby LSP is being initialized
THEN "undefined method `key_for' for Bundler::Settings:Class (NoMethodError)" error occurs
AND Ruby LSP fails to initialize

GIVEN ruby project with Bundler version >= 2.2.17
WHEN Ruby LSP is being initialized
THEN no errors occur
AND Ruby LSP initializes successfully

Attaching an error example that is currently happening: undefined_method_key_for_for_Bundler_Settings_Class_NoMethodError.txt.

Expected Result:

GIVEN ruby project with Bundler version < 2.2.17
WHEN Ruby LSP is being initialized
THEN no errors occur
AND Ruby LSP initializes successfully

GIVEN ruby project with Bundler version >= 2.2.17
WHEN Ruby LSP is being initialized
THEN no errors occur
AND Ruby LSP initializes successfully

@Artes02 Artes02 requested a review from a team as a code owner February 8, 2025 10:33
Copy link

graphite-app bot commented Feb 8, 2025

How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@Artes02 Artes02 force-pushed the fix-backward-compatibility-for-bundler-settings branch from ef292a0 to 4387de6 Compare February 8, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant