-
Notifications
You must be signed in to change notification settings - Fork 45
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
pygls update #86
pygls update #86
Conversation
Wow, this looks great! At a glance, I have no major issues with the direction you've taken this. I'll wait for pygls to release the next version and then immediately start testing out this PR / giving you any functionality-based feedback that I find. Thanks for pushing this ahead, I'm very pleased with all the simplifications you've produced! |
@dimbleby looks like pygls was just released, let me know when you believe this is mergeable and I'll so some deeper tests then. |
I re-pushed to use pygls 0.10.0, should be good for a closer look now. |
actually I've made a tiny mess of rewriting the history and my inner perfectionist isn't quite happy, will fix shortly. |
I've just noticed pygls_type_overrides.py. That looks as though it might be workaround for things that no longer need workaround, though I'm not sure exactly what. Can we remove that too, do you think? |
Yes, looks like this went in for #38 and was a particular case of the thing where pygls liked to send |
Oh, and also the private reimplementation of |
So we only need to do our own parsing for initialization options
This PR relates to openlawlibrary/pygls#151 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Not for merging, certainly until pygls next releases.
I've been wondering since #78 (comment) whether openlawlibrary/pygls#139 was going to be a good thing, and at the weekend I kind of got sucked into going down the path of finding out.
This - and prompting that project to merge their longstanding PR - is the result.
Obviously not for merging until there's an actual pygls release and pyproject.toml can be updated to point at that, rather than to a local checkout. But I figured I should get this in front of you sooner rather than later in case you hate it, or want changes, or were about to duplicate the work, or whatever.
I made a series of commits:
What else interesting? Oh, bumped min python version to 3.6.1, some of our dependencies need that to be able to update to recent versions and I think it should be uncontroversial: surely very few even of those who are stuck on python 3.6 are stuck on python 3.6.0.