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

Reload the LS when relevant config changes #499

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

jribbink
Copy link
Contributor

@jribbink jribbink commented Dec 31, 2023

Closes #494

Description

  • Some refactoring to the Settings class (it was kind of bloated and all of the stuff for missing values, defaults, etc. is useless since it's handled by VSCode itself). The singleton was also kind of useless because we were injecting instances everywhere anyways and only complicates testing
  • Adds a reload to LS when LS-related configuration changes (important for Cadence 1.0, since users will be changing cadence.flowCommand setting)
  • Changes test concurrency setting to test.maxConcurrency instead. This was an oversight when I originally added the feature, it probably makes sense to start getting in the habit of namespacing properly.

Imo if specific settings require additional transformations, etc. we can move these to their own sub-providers but the top-level settings provider should be more generalized


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@jribbink jribbink requested a review from nialexsan January 2, 2024 16:45
Copy link
Contributor

@nialexsan nialexsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jribbink jribbink force-pushed the jribbink/reload-ls-config-change branch from 54556fa to 0d07147 Compare January 3, 2024 17:30
@jribbink jribbink merged commit 3398955 into master Jan 3, 2024
5 of 6 checks passed
@jribbink jribbink deleted the jribbink/reload-ls-config-change branch January 3, 2024 17:31
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.

LS does not reload when changing cadence.flowCommand
2 participants