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

Move utility functions to dedicated space #1074

Merged
merged 2 commits into from
Apr 26, 2022
Merged

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Apr 25, 2022

This moves all the utility files to a dedicated folder.

@jpogran jpogran added this to the 2.23.0 milestone Apr 25, 2022
@jpogran jpogran self-assigned this Apr 25, 2022
@jpogran jpogran marked this pull request as ready for review April 25, 2022 16:43
@jpogran jpogran requested a review from a team as a code owner April 25, 2022 16:43
This moves all the utility files to a dedicated folder.
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

@jpogran jpogran merged commit e3a9edd into main Apr 26, 2022
@jpogran jpogran deleted the organize-util-functions branch April 26, 2022 14:27
jpogran added a commit that referenced this pull request Apr 28, 2022
This extracts the LanguageClient creation from ClientHandler and puts it directly inside the activation point.

In order to implement hashicorp/terraform-ls#722 we need to add a new StaticFeature to register a client-side command for terraform-ls to call when it knows the `terraform.providers` view needs to be refreshed.

StaticFeatures are registered using the LanguageClient.registerFeature method, which means the ClientHandler class needs to create the StaticFeature. The StaticFeature needs both the LanguageClient and `terraform.providers` view created before it can be initialized. The LanguageClient is created by the ClientHandler class. This all results in a cyclic dependency.

We could resolve this cyclic dependency by adding more responsibility to ClientHandler, but this introduces more complexity to the class. This will become increasingly hard to deal with as more aspects like StaticFeature are added.

We resolve this by extracting the creation of LanguageClient and moving dependent features like the StaticFeatures to the main activation method. This puts all the parts that rely on each other in the same place where the data needed is located to make decisions about whether they are activated or not.

This builds on work done in:

- #1073
- #1074
- #1075
- #1079
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants