Make monaco workspace disposable - #330
Conversation
|
@mofux @kaisalmen currently, there no way to uninstall the services it means we can't change the rootUri without reloading the page |
|
Do you mind adjusting the example? Couldn't we demonstrate the functionality via a reset button: Dispose and re-init, even if no new url is used. WDYT? |
|
Do you think it will be relevant? Nothing will happen in the interface, and we can already achieve the reset button without this feature (we don't need to uninstall the services) |
|
The "simple" example is likely not the best place for this. The return value of |
kaisalmen
left a comment
kaisalmen
left a comment
There was a problem hiding this comment.
Code does not compile without adding new imports, otherwise this is fine. Thanks.
|
It's still not clear what is the new process here? am I supposed to merge it? Who is responsible for making a new release? |
|
We should not automatically list all for review, here. I need see how to limit this it to at least one person. You and me can anyway merge with admin privileges. So, if you are ok with one one person reviewing, then go ahead and merge. As this is a breaking change we should release a 0.19.0. How urgent is this for you? I am working on upgrading build and dependencies right now, but this won't be finished today. |
|
This is not really urgent, I just wanted to get rid of a workaround Btw: What about adopting semantic release here? |
👍 enforced or relaxed via humans? What do you have in mind? |
|
The more it is automatic the better I think The most important part is using the commit message convention allowing to detect if the next version should be a patch/minor/major version |
|
Ok from my side. Shall I merge now? It is a breaking change and should not be release before 0.19.0. I would like it to include this and at least build/dep. update. Are you ok with waiting until next release until all is done? A |
What if I split this one into 2 PRs: one with the workspace being disposable and one with the MonacoServices.install returning a disposable (which is the breaking change)? (to be able to merge the first one - the one I want) |
|
Forget about it, it can wait |
|
We can merge it now and deal with bugfix releases if needed. No worries. The other stuff should not take weeks. |
|
I will rebase this branch due to #340 and then we can merge it as well. |
d53522e to
2ce376e
Compare
|
Will there be a 0.19.0 soon? btw, shouldn't it be a 1.0.0? |
Yes, soon, but maybe after Easter weekend. I already released a Btw, I just enabled https://github.com/TypeFox/monaco-languageclient/discussions We can use this to discuss things that have a broader scope than a single issue. |
|
ok! I'm just not a big fan of not being able to release a fix at any time if we need to |
|
Yes, I understand that. We can do it earlier and integrate further updates/improvements later. Did you test the new build and tried the examples yourself? |
|
Ok I will, that's not that easy. Didn't you claim the code was unchanged? ^^ |
|
Yes 🙂 But almost the whole example explanation/start-up changed 😅 |
|
I noticed an issue, I'll make a PR Btw, isn't it the moment to switch to the |
|
Regarding the examples: that's a shame we have to build it before starting it, can't we use ts-node instead? |
|
Both clients examples are served by vite and don't require a build, but the node example requires one. |
|
So what do you think about using ts-node? We just need to replace |
Yes, go-ahead make a PR 👍 |
|
please check if README requires adjustments |
2 participants
This include a breaking change: Monaco.install now return a Disposable instead of the services