Open
Conversation
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The changes to gem pin caching introduced bugs in use cases where Solargraph is not running in a bundled environment.
My first approach in #1178 was to detect whether the runtime environment has a bundle configuration and fall back to external bundles. This partially fixed the problem, but did not address use cases where the workspace also does not have a bundle configuration. It also exposed other bugs elsewhere in caching processes, most notably deadlocked threads.
This PR reverts to the caching processes mostly as they exist in v0.58.3. Additional
Shellspecs assert that caching commands work when Solargraph is running in an unbundled environment and operating on an unbundled workspace. These processes are a prime candidate for refactoring, but we'll have to take a more cautious approach to avoid regression bugs.In the future, we should also be more cautious in how we handle release branches. First, when we create a release branch (e.g.,
v0.59), we should immediately open a PR for merging it to master. This gives us a quick reference for failing checks and merge conflicts during development. Second, we should introduce iterative PRs instead of large changes that are difficult to review and disentangle if necessary.