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.

Sign up for GitHub

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

Jump to bottom

async server #109

Merged
merged 1 commit into from
Jan 30, 2022
Merged

async server #109

merged 1 commit into from
Jan 30, 2022

Conversation

bung87
Copy link
Contributor

@bung87 bung87 commented Jan 8, 2022 •
edited
Loading

hmm , as changed main branch github can't not really diff like other tools, please just check async part.

@bung87
Copy link
Contributor Author

bung87 commented Jan 29, 2022

any news ?

@PMunch
Copy link
Owner

PMunch commented Jan 29, 2022

No, I've been busy with FOSDEM so I haven't had time to review this yet.

@bung87
Copy link
Contributor Author

bung87 commented Jan 30, 2022

anyone else can review ? I wanna finish my PRs during Spring Festival

@PMunch
Copy link
Owner

PMunch commented Jan 30, 2022

Unfortunately I'm the only one with the required amount of familiarity with the codebase. I unindented the main procedure though just to get a better idea of the actual changes and this looks good so I'll merge it. Not entirely sure if making this async will actually have any benefit at all though, the major bottleneck is not the read/write from stdin/stdout but rather waiting for the compiler.

@PMunch PMunch merged commit 50abfa3 into PMunch:master Jan 30, 2022
@bung87
Copy link
Contributor Author

bung87 commented Jan 30, 2022

anyway thanks, I will create rest of PRs soon.

@PMunch
Copy link
Owner

PMunch commented Jan 30, 2022

Please note that I added another commit to fix up some style things, so please rebase them on the current master :)

This was referenced Jan 30, 2022
Merged
Merged
Open
@PMunch
Copy link
Owner

PMunch commented Jan 31, 2022

It seems the test is not working from this PR, please fix it

ynfle added a commit to ynfle/nimlsp that referenced this pull request Jan 31, 2022
@ynfle
Tests: upgrade tests to use new async design
0a965b2 
Addresses PMunch#109 (comment)
@ynfle ynfle mentioned this pull request Jan 31, 2022
Merged
ynfle added a commit to ynfle/nimlsp that referenced this pull request Jan 31, 2022
@ynfle
Tests: upgrade tests to use new async design
b82037f 
Addresses PMunch#109 (comment)
ynfle added a commit to ynfle/nimlsp that referenced this pull request Jan 31, 2022
@ynfle @PMunch
Tests: upgrade tests to use new async design
688341b 
Addresses PMunch#109 (comment)

Co-authored-by: PMunch <peterme@peterme.net>
PMunch added a commit that referenced this pull request Jan 31, 2022
@ynfle @PMunch
Tests: upgrade tests to use new async design (#116)
2d9ecc6 
Addresses #109 (comment)

Co-authored-by: PMunch <peterme@peterme.net>

Co-authored-by: PMunch <peterme@peterme.net>
@williamboman williamboman mentioned this pull request Feb 7, 2022
Closed
1 task
muxueqz pushed a commit to muxueqz/nimlsp that referenced this pull request Mar 1, 2022
@bung87 @muxueqz
async server (PMunch#109)
e9fc54a
muxueqz pushed a commit to muxueqz/nimlsp that referenced this pull request Mar 1, 2022
@ynfle @PMunch @muxueqz
Tests: upgrade tests to use new async design (PMunch#116)
7b92fa0 
Addresses PMunch#109 (comment)

Co-authored-by: PMunch <peterme@peterme.net>

Co-authored-by: PMunch <peterme@peterme.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bung87 @PMunch