Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Fixes #833 Move plugin-config outside of tribe cmds #845

Merged
merged 1 commit into from
Apr 13, 2016
Merged

Fixes #833 Move plugin-config outside of tribe cmds #845

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions cmd/snapctl/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,21 @@ var (
flRunning,
},
},
{
Name: "config",
Copy link
Contributor

Choose a reason for hiding this comment

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

should you use global-plugin-config as the name or something to make it clear to users that it only lists the global plugin config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. What other configs are there where the target is a plugin?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine as it has plugin in the front of the command and is consistent with others. Also this command only shows the global config. Do I understand this correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I see how showing anything else would make sense. It's displaying the output from /v1/plugins/:type/:name/:version/config rest call. Are there other config values that would make sense to show for a plugin?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we can define plugin specific config inside task manifest although that's task specific. If answer is yes, how that will have effect on the global plugin config. This topic may not related to your fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

your fix is good! LGTM 👍

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 put plugin specific stuff inside task manifest but I think that would fall under task config. Maybe there should be a way to see that like: snapctl task config <task-id>?

Copy link
Contributor

Choose a reason for hiding this comment

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

a nice doc for the current state and the future state, that would be nice. It has nothing to do with your fix here. I'm just trying to understand it. thanks.

Subcommands: []cli.Command{
{
Name: "get",
Usage: "get <plugin_type>:<plugin_name>:<plugin_version> or get -t <plugin_type> -n <plugin_name> -v <plugin_version>",
Action: getConfig,
Flags: []cli.Flag{
flPluginName,
flPluginType,
flPluginVersion,
},
},
},
},
},
},
{
Expand Down Expand Up @@ -198,22 +213,6 @@ var (
},
},
},
{
Name: "plugin-config",
Usage: tribeWarning,
Subcommands: []cli.Command{
{
Name: "get",
Usage: "get <plugin_type>:<plugin_name>:<plugin_version> or get -t <plugin_type> -n <plugin_version> -v <plugin_version>",
Action: getConfig,
Flags: []cli.Flag{
flPluginName,
flPluginType,
flPluginVersion,
},
},
},
},
}
)

Expand Down