Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Fix inconsistency in plugin name #452

Merged
merged 11 commits into from
Dec 3, 2019
Merged

Conversation

superboyiii
Copy link
Member

Fix inconsistency between Plugin name and Plugin class name. For example: SimplePolicy is using SimplePolicyPlugin as class name, but using SimplePolicy as plugin name. This issue also could be seen on RpcSystemAssetTracker. I think it's also necessary to unify plugin class name on neo3 to avoid this issue.

Fix inconsistency between Plugin name and Plugin class name. For example: ```SimplePolicy``` is using ```SimplePolicyPlugin``` as class name, but using ```SimplePolicy``` as plugin name. This issue also could be seen on ```RpcSystemAssetTracker```. I think it's also necessary to unify plugin class name on neo3 to avoid this issue.
Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the name of the class is used?

@superboyiii
Copy link
Member Author

superboyiii commented Aug 29, 2019

But the name of the class is used?

Hi, Shargon. What do you mean? Yes the class is used, but uninstall can't be successful if you add this check. I think the best solution is to unify class name to plugin project name. But I'm not sure if it will influence a lot or not.

@shargon
Copy link
Member

shargon commented Sep 9, 2019

For reproduce the error, what are the steps?

install plugin RpcSystemAssetTracker uninstall plugin RpcSystemAssetTracker
?

@superboyiii
Copy link
Member Author

superboyiii commented Sep 10, 2019

For reproduce the error, what are the steps?

install plugin RpcSystemAssetTracker uninstall plugin RpcSystemAssetTracker
?

Yes. Now only SimplePolicy and RpcSystemAssetTracker have this issue.
For example install RpcSystemAssetTracker, uninstall RpcSystemAssetTracker.

@shargon
Copy link
Member

shargon commented Sep 10, 2019

Check my last changes, it works for me when is loaded. But the problem is how to remove a used plugin.

@shargon
Copy link
Member

shargon commented Sep 10, 2019

mmmm... i see two solutions, as you said.

  • 1 Remove plugin.Name, and use the library name for the plugin
    • This avoid create two plugins in the same library
  • 2 Use a file like plugins.json for store what plugins are installed.
    • This help when we need to remove hot-plugins (currently used)

@superboyiii
Copy link
Member Author

mmmm... i see two solutions, as you said.

  • 1 Remove plugin.Name, and use the library name for the plugin

    • This avoid create two plugins in the same library
  • 2 Use a file like plugins.json for store what plugins are installed.

    • This help when we need to remove hot-plugins (currently used)

I think solution 1 makes sense for making plugins more regular.

@vncoelho
Copy link
Member

vncoelho commented Oct 30, 2019

@superboyiii, how is this PR? Should we merge this?
@shargon solution 1 looks more aligned with this PR, right?

@shargon
Copy link
Member

shargon commented Nov 2, 2019

1 is good for me

@shargon
Copy link
Member

shargon commented Nov 18, 2019

If we want to remove the plugin name, this should be done in neo first

erikzhang
erikzhang previously approved these changes Dec 1, 2019
erikzhang
erikzhang previously approved these changes Dec 1, 2019
@erikzhang
Copy link
Member

@superboyiii Can you test it again?

@superboyiii
Copy link
Member Author

Sure

@superboyiii
Copy link
Member Author

Here's my test steps and result.

  1. install Plugins
    image
  2. Plugins are loaded successfully
    image
  3. Uninstall plugins
    image
  4. Plugins are deleted successfully.
    image
  5. Rename one of the plugin class name, make it different as type name.
    image
  6. build it and install it
    image
  7. restart neo-cli and uninstall it.
    image
  8. Plugin is deleted successfully. Test Pass.
    image
  9. By the way, Neo v3.0.0-CI00815 isn't compatible as a dependency in neo-plugins, the code of plugins should be adjusted.
    image
  10. And if neo-cli is using dependency of Neo v3.0.0-CI00815, RpcNep5Tracker and SystemLog will break.
    image
    image
    Only these are OK.
    image

Summary: Test Pass.
Warning: RpcNep5Tracker and SystemLog need adjust if apply the latest neo on neo-cli or neo-plugins.

@erikzhang erikzhang merged commit 15e52a0 into master Dec 3, 2019
@erikzhang erikzhang deleted the Fix-Plugin-Inconsistency branch December 3, 2019 07:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants