Skip to content
This repository has been archived by the owner on Jul 3, 2018. It is now read-only.

fix generator to be consistent with current plugin system #2

Merged
merged 2 commits into from
May 4, 2018

Conversation

akurinnoy
Copy link
Contributor

This PR updates the plugin generator to be consistent to theia-demo-plugins/wiptheia#1

Signed-off-by: Oleksii Kurinnyi okurinny@redhat.com

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
@@ -16,7 +16,7 @@ export function doStartThings() {
);
}

export function doStopThings(api: typeof theia) {
export function stop(api: typeof theia) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are not sending as well theia object on stop (I forget to remove it)
it can be fixed later as well

@benoitf
Copy link
Contributor

benoitf commented Apr 20, 2018

do we need CI on this project as well ?

@akurinnoy
Copy link
Contributor Author

I think, yes

src/app/index.ts Outdated
@@ -93,6 +100,15 @@ module.exports = class TheiaPlugin extends Base {
(this.options as any).pluginName = answers.name
}

if (!(this.options as any).publisher) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if here is no publisher ? (for example I only want to create a plugin locally) ? is there a default publisher value for that case ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can omit publisher ID and it will equal an empty string.

Copy link
Contributor

@benoitf benoitf Apr 20, 2018

Choose a reason for hiding this comment

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

should publisher be then only a "parameter" of the generator (and not a question to user) ?
(like for theia extension where you can specify license, etc) so we don't ask the publisher information at first

it makes the flow to create the plugin as fast as possible.
We can check that publisher is valid only when we want to publish the extension

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, it is better to have a parameter.
And later it would be stored and read from .*rc file as default value.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok so we remove the question but we let "-p" option to give publisher and later we'll implement *.rcstuff ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes,
I will add a fix

@benoitf benoitf merged commit f2ad9d9 into master May 4, 2018
@benoitf benoitf deleted the update-generator branch May 4, 2018 05:36
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.

2 participants