The future of the Bazel LSP

facebook / starlark-rust

A Rust implementation of the Starlark language

Apache License 2.0

699 stars 57 forks source link

The future of the Bazel LSP #104

Closed cameron-martin closed 5 months ago

cameron-martin commented 9 months ago

I recently submitted a few patches to the Bazel LSP (#100, #101, #103). However, this is intended to be merely a proof of concept and there are some larger changes that need to be made (e.g. adding tests, bzlmod support). There was some discussion on the future of this project in #51, and the conclusion seemed to be to eventually split it out into a separate project and maintained by people with more of a stake in Bazel, but only when the interface is stable enough.

Is the interface stable enough to pull this out yet?
If not, are you willing to accept contributions to this even though it is meant as a proof of concept, especially larger ones?
If the interface isn't stable yet, does it make sense to keep the current PoC in this repo to verify changes to the interface, along with a more fleshed-out implementation in a separate repo?

ndmitchell commented 9 months ago

I'm going to review the patches when I get back to work tomorrow. My guess is these patches are fine, but that adding tests/bzlmod support is probably going too far? Curious what @stepancheg thinks, especially about the stable interface question.

ndmitchell commented 8 months ago

My thoughts are to make the next steps (testing with Bazel, significant complexity) it's probably best to pull it out as a separate project. I don't think there's much value to leaving stale code in this repo as a test - we can just include a pointer to your code, and we do co-develop the LSP with the Buck2 usage of it, so it's not going to break. The interface is relatively stable, but it might be best to git submodule your Bazel LSP to avoid requiring releases if you do need to tweak the interface?

But keen to hear dissenting views, @stepancheg

cameron-martin commented 8 months ago

I've pulled out the Bazel implementation here, where I am continuing any modifications: https://github.com/cameron-martin/bazel-lsp

Maybe in the future the bazelbuild org can inherit this.

cameron-martin commented 5 months ago

I'll close this now, since bazel-lsp has advanced significantly past the bazel language server in this repo!

stepancheg commented 5 months ago

@cameron-martin bazel-lsp readme is a bit misleading: it says "forked from starlark-rust", but it is actually just extends starlark-rust LSP for bazel. Up to you.

Also, I guess now we should remove Bazel support from starlark-rust repo and maybe point to bazel-lsp instead?

cameron-martin commented 5 months ago

I wanted to attribute the original authoring of this bazel-specific extension of the language server to this repo, but yeah possibly a bit misleading...

Maybe saying "based on starlark-rust" is sufficiently vague to not be misleading?

stepancheg commented 5 months ago

I wanted to attribute the original authoring of this bazel-specific extension

This is not us, we accepted PRs, but we don't use Bazel. I think that was @MaartenStaa from digging git log.