[go: nahoru, domu]

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

Initialize Language Server on Client Connection #1535

Merged
merged 12 commits into from
Mar 4, 2021

Conversation

4e6
Copy link
Contributor
@4e6 4e6 commented Mar 2, 2021

Pull Request Description

close #1403

Lazy initialization of the Language Server.

Changelog:

  • add: InitializationComponent containing the init logic
  • update: MainModule does not create the directories or initializes the databases, or the truffle context
  • update: JsonConnectionController runs initialization logic when receiving the InitProtocolConnection message

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All documentation and configuration conforms to the markdown and YAML style guides.
  • All code has been tested where possible.

@4e6 4e6 added Type: Enhancement p-high Should be completed in the next sprint labels Mar 2, 2021
@4e6 4e6 self-assigned this Mar 2, 2021
@4e6 4e6 requested a review from lolczak March 2, 2021 13:33
@4e6
Copy link
Contributor Author
4e6 commented Mar 3, 2021

Had to remove the failing test because it assumes that the language server is initialized on project/open command. Not proud of that, but I wasn't able to come up with the idea of how to make it work 😐

Copy link
Contributor
@lolczak lolczak left a comment

Choose a reason for hiding this comment

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

Great! Please use Future.sequence instead of fold. Can you roll out a new release candidate?

Comment on lines +15 to +17
resources.foldLeft(Future.successful(InitializationComponent.Initialized)) {
(action, resource) => action.flatMap(_ => resource.init())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Future has a combinator for that Future.sequence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Future.sequence takes Seq[Future], meaning that all futures are started at once. I need sequential behavior, when the next action starts after the previous is complete.

Copy link
Contributor
@iamrecursion iamrecursion left a comment

Choose a reason for hiding this comment

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

I'm only approving the changes to .github/settings.yml.

Copy link
Member
@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

I've only reviewed the CI config.

It looks good overall, but please update also the legal-review.yml to be consistent.

Also release-update-broken.yml must definitely be updated because it also uses aws cli.

Copy link
Member
@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

CI config looks good

@4e6 4e6 mentioned this pull request Mar 4, 2021
@iamrecursion iamrecursion merged commit b5e9895 into main Mar 4, 2021
@iamrecursion iamrecursion deleted the wip/db/init-language-server branch March 4, 2021 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p-high Should be completed in the next sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Language server lazy initialization
4 participants