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

Consider merging with lspower and maintaining a single code base #308

Closed
silvanshade opened this issue Feb 16, 2022 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@silvanshade
Copy link
Contributor

silvanshade commented Feb 16, 2022

Hi @ebkalderon, I wanted to start a discussion about merging some of the changes I made when lspower was forked and then deprecating that project so we can maintain a single code base. (I think you pinged me about this but I don't see where now so I'll just create this new issue).

I think the only significant changes since you've been updating tower-lsp again are:

  1. The use of async-codec-lite in order to facilitate wasm targets re: tower-lsp in the web #187
  2. The use of httparse and twoway instead of nom. This is more efficient and reduces the number of dependencies.

A lot of the rest of the changes are either bug fixes or updates you've already made or stylistic changes which aren't important.

I could work on PRs for the above two and, deprecate lspower, and then help to maintain tower-lsp going forward if that is something that interests you.

@ebkalderon
Copy link
Owner

Hello again, @silvanshade! It's great to hear from you again. Thanks so much for maintaining lspower in my absence and submitting excellent improvements to the codebase all the while.

Sure, I would be happy to merge in those two changes into tower-lsp and resume the project in a more full capacity, if you're okay with that! Although small note: the twoway crate is technically considered deprecated today and advises users to go for memchr instead. This should still be fewer dependencies than relying on nom, though, either way. I'd also be happy to grant you some rights to the repository, if you're interested, considering your excellent contributions so far.

I'm looking forward to rolling out the custom client-to-server request approach described in #256 (comment), as well as addressing #284 long-term (I think I may have a vague idea of how to proceed and will update the issue shortly), and in light of this, I think it would be wonderful if the two codebases were consolidated together.

Feel free to get started on those PRs and we can review them together! And please freely ping me if you have further questions or suggestions as well. ❤️

@ebkalderon ebkalderon added the enhancement New feature or request label Feb 17, 2022
@ebkalderon
Copy link
Owner

Just merged in the changes, @silvanshade! What would you say to receiving collaborator access to this repo, adding you as an author to the Cargo.toml, and possibly adopting the lspower name unilaterally to try to unify the user bases of both crates?

@silvanshade
Copy link
Contributor Author

@ebkalderon thanks for the follow up! That all sounds good to me. I'm not entirely sure if it would be better to use lspower or just keep the name tower-lsp. I don't really have a strong opinion about it either way so I guess it's up to you but it's fine with me if you think that would be best.

@silvanshade
Copy link
Contributor Author

So one minor point in favor of keeping the tower-lsp name is that I already created a cancellation token API for lspower but it's not as nice as the one you described recently. So moving to that would be a breaking change I guess. I kind of doubt anyone is actually using it but still.

@ebkalderon
Copy link
Owner

That's certainly a fair point. I wouldn't want to cause confusion for existing lspower and tower-lsp users. I guess we could shift focus towards #207 and choose a new name instead, once the API is finally stabilized.

@ebkalderon
Copy link
Owner

Now that version 0.16.0 is up on crates.io, this task is effectively complete. 🎉 I've personally added you, @silvanshade, as a co-author of the tower-lsp crate in the Cargo.toml. If you would like collaborator access to the repository going forward, please feel free to reach out to me personally. Thanks again for your great co-maintainership and contributions so far!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants