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

Ensure ancient Ruby compatibility in vscode/activation.rb #3250

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

sambostock
Copy link
Contributor

This ensures if the version is bumped in development, it is also added
to the matrix.

Motivation

vscode/activation.rb is run by the extension to collect Ruby version information.

This means it may run against an unknown Ruby version, which means we can't rely on recent syntax/method support. We want the script to run without error even on these old versions, so that we can correctly detect the Ruby version and detect if we support it.

Implementation

This replaces filter_map with .map { ... }.compact, which ensures the activation script doesn't blow up on old Ruby versions.

It also configures CI to check for this to ensure no regressions are introduced.

Automated Tests

Yes, see above ☝️

Before Merging

  • Clean up commits after PRs this bases off of are merged

This ensures if the version is bumped in development, it is also added
to the matrix.
We should always test against the minimum Ruby version we support, and
it shouldn't be removed without also bumping the `required_ruby_version`
in the gemspec.
- Bundler 2.6 requires Ruby 3.1
- Tapioca > 0.14 requires Ruby 3.1

I've changed `BUNDLED WITH` to the latest version in the 2.5 series,
which supports Ruby 3.0, and I've removed the version requirement on
`tapioca` (and run `bundle update`).

Let's see if this passes CI...
This identifies a number of offenses, which have been corrected.

Notably, `rubocop-md` treats Markdown code blocks without a language
specified as Ruby by default. We could change this behaviour, but it's
clearer to explicitly mark the snippets as `txt` when there is no
matching language.
Given the purpose of this script is to collect information about the
user's Ruby version, we don't know their version yet. Therefore, we must
write the most backwards compatible Ruby we can, or else they may get
errors when the editor's environment includes an older Ruby version than
we expect (e.g. an old system Ruby, in a non-Ruby project).
Given the purpose of this script is to collect information about the
user's Ruby version, we don't know their version yet. Therefore, we must
write the most backwards compatible Ruby we can, or else they may get
errors when the editor's environment includes an older Ruby version than
we expect (e.g. an old system Ruby, in a non-Ruby project).

Running the script in Ci ensures we are compatible at runtime, not just
as a matter of syntax compatibility.
`filter_map` was introduced in Ruby 2.7, but this script may be run by
an ancient Ruby version.
Copy link

graphite-app bot commented Feb 26, 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.

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