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

Programmatically add labels #137

Merged
merged 4 commits into from
Nov 25, 2020
Merged

Programmatically add labels #137

merged 4 commits into from
Nov 25, 2020

Conversation

hanzei
Copy link
Collaborator

@hanzei hanzei commented Nov 4, 2020

Summary

Instead of storing all labels in the DB we now programmatically generate the list using metadata stored in the DB file. That ensure there is a single source of truth. The stored metadata will later also be used for filtering on the client side.

Given that model.Plugin in the store can't be send to the client any longer because the labels have to get added, I implemented a AddLabels() method that takes care of this. I'm unsure where the call it it belongs. Right now it's called after the plugins are read from store.

I thought about spiting model.Plugin into a two structs: A stored plugin and a returned plugin. E.g. Labels should not be stored and only returned and Platforms should not be returned via the API. But that seamed like to big change to piggy back into this PR. Still, I'm wondering if it's a valuable change.

Migration was done via the migration command. No manually edits.

Ticket Link

https://mattermost.atlassian.net/browse/MM-30240

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Nov 4, 2020
Comment on lines 40 to 41
Maintainer Maintainer `json:"maintainer"` // The maintainer of the plugin
Stage Stage `json:"stage"` // The stage in the software release cycle that the plugin is in
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm 1/5 on the name Maintainer and 0/5 on Stage. Suggestions are welcome!

@lieut-data lieut-data changed the title Problematically add labels Programmatically add labels Nov 4, 2020
@lieut-data
Copy link
Member

@hanzei, can you elaborate on the rationale for making this change? I see "single source of truth", but I'm not sure what the "other" source of truth was in this case. Happy to continue reviewing, but just want to make sure I understand why we want to start modelling these properties directly instead of keeping them flexible as labels. Could we just filter client-side on the labels vs. hard-coding a particular schema that might have to change in the future?

Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

I had a chance to sync up with @hanzei offline on the high-level questions -- changes look good!

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Nov 25, 2020
@hanzei hanzei merged commit c12f501 into master Nov 25, 2020
@hanzei hanzei deleted the MM-30240_labels branch November 25, 2020 11:02
cpoile added a commit that referenced this pull request Mar 15, 2023
* Programmatically add labels (#137)

* Add notice about Looker to plugin checklist (#151)

* Add Experimental and Partner label (#145)

* Add experimental and partner label

* Update generator add

* Apply labels before sending a response

* [MM-32593] Update golangci-lint to v1.33.2 (#165)

* [MM-32278] Change "E20 Only" tag on listings to "E20 and Cloud" (#161)

* Change EE label name (#168)

* [MM-33116] Use embed package to include Marketplace plugin list for lambda (#170)

* Merge production into master (#184)

Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>
Co-authored-by: Christopher Speller <crspeller@gmail.com>
Co-authored-by: Daniel Espino García <larkox@gmail.com>
Co-authored-by: Alejandro García Montoro <alejandro.garciamontoro@gmail.com>
Co-authored-by: Christopher Poile <cpoile@gmail.com>
Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>

* Update add_plugin.md

* Update add_plugin.md

* Update add_plugin.md

* [MM-39133] Update Marketplace labels for new SKUs (#224)

* Update add_plugin.md

* Update add_plugin.md

* Update icon URL from mattermost.org to mattermost.com (#269)

* change "Enterprise and Cloud" to "Professional/Enterprise"

* gofmt

* Change "license to run" to "subscription"

Co-authored-by: Jason Blais <13119842+jasonblais@users.noreply.github.com>

* Use mattermost-server/v6 (#283)

* Correctly set ReturnAllVersions and PluginID for upstream requests (#268)

* MM-49609 - Update mattermost-server version (#333)

* update mattermost-server version

* remove ioutil

* remove deprecated linters, add revive

* experimenting with go and node circleci upgrades

* fix

* fix

* go 1.19, revive, fix new linter errors

* fix golangci-lint-version to match dep orb

* fix comment

* use same golangci-lint as plugins

* fix failing build?

* testing...

* remove unneeded gotest/tools

* fix

* commit the plugins.json differences now, so next update doesn't have to

* MM-50203 - Update serverless (#338)

* Allow adding plugins with an invalid manifest to the Marketplace (#340)

* Downgrade node version to 12 (#343)

* Always pull tags from master (#344)

---------

Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>
Co-authored-by: Christopher Speller <crspeller@gmail.com>
Co-authored-by: Daniel Espino García <larkox@gmail.com>
Co-authored-by: Alejandro García Montoro <alejandro.garciamontoro@gmail.com>
Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com>
Co-authored-by: Michael Kochell <mjkochell@gmail.com>
Co-authored-by: Jason Blais <13119842+jasonblais@users.noreply.github.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants