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

Add first experiments showing basic autocompletion and token support #5889

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Nov 14, 2023

These are my first experiments/attempts of a Bnd Language server implementation migrated to bnd see

for client side, this would require some adjustment to bndtools source editor but might better be a separate PR...

@pkriens
Copy link
Member

pkriens commented Nov 15, 2023

We never use the copyright headers since they are kind of annoying at the top and we seem to have been able to convince even IBM they were needed. They also need to be policed because they could say something different than our license we maintain at the top.

We generally also do not put our names in source files. Almost all code becomes touched by others or is copied from others. We have the log in git, with the signed commits, so I do not see the use of it and prefer not to start this imho awkward convention.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 15, 2023

Headers removed...

@pkriens
Copy link
Member

pkriens commented Nov 16, 2023

You want me to automatically merge these?

@laeubi
Copy link
Contributor Author

laeubi commented Nov 29, 2023

Would be good so others can build on top of it.

@pkriens
Copy link
Member

pkriens commented Nov 29, 2023

Yes, but it would become part of #5861

We now have two drafts for the same purpose.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 29, 2023

I don't understand this targets the branch you mentioned so it will be merged there, and then this one will vanish so whats wrong with that?

@pkriens
Copy link
Member

pkriens commented Dec 1, 2023

We now have two PRs for the same work. This is confusing.

The primary PR tries to merge the bndtools:feature/lsp branch on master. I want that so we can track difference with master.

Your PR tries to merge with the bndtools:feature/lsp branch, so there is no comparison to master. So if we can merge yours, we are back to 1 PR where we can have discussions.

@laeubi
Copy link
Contributor Author

laeubi commented Dec 1, 2023

We now have two PRs for the same work. This is confusing.

No, my PR is extending your PR ...

So if we can merge yours, we are back to 1 PR where we can have discussions.

That's how feature branches work, and you wanted a feature branch for that ... I can't merge it as I'm not a repro owner and is is usually good to review changes against the feature branch if working with different people on the same feature so I'm not understand why/what should be made different here?

@pkriens
Copy link
Member

pkriens commented Dec 1, 2023

No, my PR compares the branch against master. So I want it to continuously follow. I want your changes in that PR and then your PR closed.

First time I tried this but I want to keep the list of PRs as small as possible. And this is a working branch so there is no use to keep a draft. That is why I want to merge it to the branch.

@laeubi laeubi marked this pull request as ready for review December 1, 2023 17:11
@laeubi
Copy link
Contributor Author

laeubi commented Dec 1, 2023

I set it to "ready" all checks are green, but its up to you to press the merge button:

Only those with write access to this repository can merge pull requests.

@pkriens pkriens merged commit 94a27bd into bndtools:feature/lsp Dec 8, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants