Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Refactor Ruby and output channel #874

Merged
merged 3 commits into from
Nov 6, 2023
Merged

Refactor Ruby and output channel #874

merged 3 commits into from
Nov 6, 2023

Conversation

vinistock
Copy link
Member

Motivation

I think there are a few opportunities to improve the code quality in our extension, make it easier to test and better prepare to support Shopify/ruby-lsp#1510.

This PR makes some refactors on Ruby and our output channel.

Implementation

I recommend reviewing by commit.

For Ruby, there are basically three changes:

  • Started requiring the workspace folder path (preparation for Support using the LSP with multiple projects in the same VS Code instance ruby-lsp#1510)
  • Switched to using async file checks everywhere
  • Started avoiding a second shell out to Ruby for fetching RUBY_VERSION and YJIT information. The only version manager where this is not possible is shadowenv, which has a different activation mechanism. For shadowenv, we continue doing the same

For the output channel, the changes are:

  • Making it a log channel. This works basically like a Rails logger and is much nicer than what we had before. It includes timestamps and log level
  • Put it in a constant, since the channel has no dependencies and we want every part of the extension to always reuse the same one

Started putting common functionality in a common.ts file. I will follow up with some more refactors that will likely move more stuff in there and simplify the existing classes.

Automated Tests

Fixed tests where necessary.

@vinistock vinistock self-assigned this Nov 3, 2023
@vinistock vinistock requested a review from a team as a code owner November 3, 2023 20:10
@vinistock vinistock requested review from andyw8 and st0012 November 3, 2023 20:10
Base automatically changed from vs/alert_shadowenv_users_about_missing_ruby to main November 6, 2023 15:06
src/extension.ts Outdated Show resolved Hide resolved
src/ruby.ts Outdated Show resolved Hide resolved
@vinistock vinistock merged commit 1ba7b0d into main Nov 6, 2023
4 checks passed
@vinistock vinistock deleted the vs/refactors branch November 6, 2023 18:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants