-
Notifications
You must be signed in to change notification settings - Fork 283
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
fix: added a dummy package #1527
fix: added a dummy package #1527
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1527 +/- ##
=======================================
Coverage 70.04% 70.04%
=======================================
Files 368 368
Lines 14108 14108
Branches 1701 1701
=======================================
Hits 9882 9882
Misses 3298 3298
Partials 928 928 Continue to review full report at Codecov.
|
a577791
to
35412e8
Compare
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.
@elenaizaguirre Quick question: Do we need the .d.ts files as well? And how about the openapi.json and the generated code files?
You see where this is going: I want to reduce the 32 files in the diff to the absolute minimum viable dummy package (which might be as low as two files, the package.json and an index.js) but I might be overlooking one (or many) things so that's why this is a question not necessarily a change request.
@elenaizaguirre Thanks for contributing. Would you add the corresponding issue on Cactus GitHub? (If there is no corresponding issue, would you post the issue?) |
35412e8
to
6c142fc
Compare
@petermetz I have deleted *.d.ts files but I think openapi.json and generated code are necessary because the first one is required for some files and generated code is used because when we instantiate a plugin from the api server, it ensures that the plugin can call some methods: const pluginPackage = require(/* webpackIgnore: true */ packagePath); // this one is our dummy package
const createPluginFactory = pluginPackage.createPluginFactory as PluginFactoryFactory;
const pluginFactoryOptions: IPluginFactoryOptions = {
pluginImportType: pluginImport.type,
};
const pluginFactory = await createPluginFactory(pluginFactoryOptions);
const plugin = await pluginFactory.create(pluginOptions);
try {
await plugin.onPluginInit(); It also calls |
@takeutak Is not an issue by itself, it is the first step for a Peter suggestion #1456 (comment). Anyway, I've added the original issue number to the description |
6c142fc
to
9a3cfc8
Compare
9a3cfc8
to
ae62d7a
Compare
@elenaizaguirre Could we just not have the plugin implement web services at all? If we limit our scope to the dummy package just being the testing instrument for verifying that the local file system path based plugin installation works then it doesn't matter if it's a web svc plugin or not IMO. I mean something like this in an index.js file: class DummyPlugin {
onPluginInit() {
console.log("DummyPlugin#onPluginInit() OK");
return Promise.resolve();
}
}
class DummyPluginFactory {
create() {
return Promise.resolve(new DummyPlugin());
}
}
module.exports.createPluginFactory = (options) => {
return Promise.resolve(new DummyPluginFactory(options));
}; |
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.
^^ See above
5e76f6e
to
357e61c
Compare
@petermetz Thanks, you were right, it was much simpler than I thought. |
357e61c
to
fe460c5
Compare
added a dummy package for tests about install packages from github sources required for hyperledger-cacti#1210 Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
fe460c5
to
c6e8d99
Compare
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.
@elenaizaguirre My pleasure and thank you very much for taking the time to address the nits as usual!
added a dummy package for tests about install packages from github
sources required for #1210
Signed-off-by: Elena Izaguirre e.izaguirre.equiza@accenture.com