Skip to content
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

Add props to plugin manifest #10124

Merged
merged 2 commits into from Jan 23, 2019
Merged

Add props to plugin manifest #10124

merged 2 commits into from Jan 23, 2019

Conversation

hanzei
Copy link
Contributor

@hanzei hanzei commented Jan 20, 2019

Summary

This PR allows plugin to include arbitrary data in the manifest. This data can be used by other plugin. My use case is the update plugin (https://github.com/hanzei/plugin-update/).

See https://github.com/hanzei/mattermost-plugin-sample/blob/master/plugin.json for an example how this looks like.

1/5 on the naming.

@hanzei hanzei added 2: Dev Review Requires review by a developer 1: PM Review Requires review by a product manager labels Jan 20, 2019
Copy link
Contributor

@levb levb left a comment

Choose a reason for hiding this comment

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

We use Props elsewhere for similar purpose, so I think the naming is consistent.

While I generally support this PR, I am 1/5 that a more flexible alternative would be to store the full manifest text "as is" in addition to unmarshaling/normalizing the known fields out of it. This way plugin developers can include custom data anywhere in the manifest, and access it later.

Copy link
Member

@jwilander jwilander left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jwilander jwilander removed the 2: Dev Review Requires review by a developer label Jan 21, 2019
model/manifest.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jasonblais jasonblais left a comment

Choose a reason for hiding this comment

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

As per above

@jasonblais jasonblais added 4: Reviews Complete All reviewers have approved the pull request and removed 1: PM Review Requires review by a product manager labels Jan 23, 2019
@jasonblais
Copy link
Contributor

Thanks! Ready to merge

@hanzei
Copy link
Contributor Author

hanzei commented Jan 23, 2019

@levb Lets keep this idea deferred until we have a use case that we can support with Props.

@hanzei hanzei merged commit 684e4cb into master Jan 23, 2019
@hanzei hanzei deleted the plugin_props branch January 23, 2019 12:46
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 23, 2019
@lindalumitchell lindalumitchell added the Tests/Not Needed New release tests not required label Feb 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed New release tests not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants