-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
plugin-ext: cleanup #8831
plugin-ext: cleanup #8831
Conversation
@benoitf I was a bit confused as to what the |
cece4f5
to
659ed6b
Compare
@marechal-p do you have tried theia frontend plug-ins as well ? |
@benoitf I tried hello world and it worked. |
cc @tsmaeder is the |
What I see from the git history, it has been introduced with the initial implementation of Plug-in system I suppose it was replaced later with
but the previous one ( /plugin ) wasn't removed.
Maybe I'm wrong... |
@azatsarynnyy I came to the same conclusion too, thanks! |
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.
I think it's safe to get rid of /plugin/:path(*)
endpoint.
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.
I think it's OK as well to get rid of it, seems to be a leftover
I test it with a couple of plug-ins which is working fine
Removed `connect` and `serve-static`. Those dependencies are not useful when we are using `express`. Their use only came because they appear in `vhost`'s documentation which is old. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
It looks like this endpoint is no longer used. Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
659ed6b
to
5ed2c70
Compare
I fixed the conflicts, thanks! |
What it does
While working on a different PR I noticed outdated things and went on to fix those.
See commit messages for more information.
How to test
Everything should work like before.
Review checklist
Reminder for reviewers