Skip to content

Conversation

@jjw24
Copy link
Member

@jjw24 jjw24 commented Jan 2, 2021

Add ability to manual reload the plugins manifest

@jjw24 jjw24 added the enhancement New feature or request label Jan 2, 2021
@jjw24 jjw24 self-assigned this Jan 2, 2021
@taooceros
Copy link
Member

Is it possible to fire up the progress bar when doing the job?

@jjw24
Copy link
Member Author

jjw24 commented Jan 2, 2021

the reload has been quick for me, is it different for you? also it is not async, so program pauses and query box is hidden

@taooceros
Copy link
Member

the reload has been quick for me, is it different for you? also it is not async, so program pauses and query box is hidden

Oh I misunderstand how this will be done, I thought it is like a query selected result. It is quick for me, since the file is not large.
However, the Task won't be waited, this runs actually async, which won't block the method that triggered it. So, potentially it will not finish even though the reloading says it has been reloaded.

Though I don't like the synchronous block, should we put a Wait() that here? Although I think loading pluginManifest won't be slow since plugins.json is small.

@jjw24
Copy link
Member Author

jjw24 commented Jan 2, 2021

Will do

@taooceros
Copy link
Member

I suddenly have a though based on this code. Regarding to #233 and the later part #195 that I wants to make the plugins to have async model so that they can utilizes the async/await model and CancellationToken from mainThread.
We can create a new async interface for plugin instead of creating default implementation for existing interface, which is not elegant and a bit dirty.
Regarding the IReloadable, we can also create a new Interface like IAsyncReloadable, like what Microsoft has added, IAsyncDisposable so that plugins won't worry about the async model they are using, and this will handling IO transmission better like the "Http.Get" here.

@jjw24
Copy link
Member Author

jjw24 commented Jan 2, 2021

I suddenly have a though based on this code. Regarding to #233 and the later part #195 that I wants to make the plugins to have async model so that they can utilizes the async/await model and CancellationToken from mainThread.
We can create a new async interface for plugin instead of creating default implementation for existing interface, which is not elegant and a bit dirty.
Regarding the IReloadable, we can also create a new Interface like IAsyncReloadable, like what Microsoft has added, IAsyncDisposable so that plugins won't worry about the async model they are using, and this will handling IO transmission better like the "Http.Get" here.

sounds good, lets do this in a seperate pr.

@jjw24 jjw24 requested review from taooceros and removed request for taooceros January 2, 2021 07:08
@taooceros
Copy link
Member

I suddenly have a though based on this code. Regarding to #233 and the later part #195 that I wants to make the plugins to have async model so that they can utilizes the async/await model and CancellationToken from mainThread.
We can create a new async interface for plugin instead of creating default implementation for existing interface, which is not elegant and a bit dirty.
Regarding the IReloadable, we can also create a new Interface like IAsyncReloadable, like what Microsoft has added, IAsyncDisposable so that plugins won't worry about the async model they are using, and this will handling IO transmission better like the "Http.Get" here.

sounds good, lets do this in a seperate pr.

Of course, I will add it as part of #195.

@taooceros taooceros merged commit d60b873 into dev Jan 2, 2021
@taooceros taooceros deleted the pm_manual_reload branch January 2, 2021 08:00
@jjw24
Copy link
Member Author

jjw24 commented Jan 2, 2021

Can we do a fresh pr? That one should just be kept for upgrading result list referring, otherwise hard to test and review

@taooceros
Copy link
Member

Can we do a fresh pr? That one should just be kept for upgrading result list referring, otherwise hard to test and review

Hmmm maybe you are right, although I originally want to solve the Explorer plugin issues and cancellation sync, so I create it based on that branch. I will rebase that toward dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants