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

[MM-57738] Retrieve Id through GetPluginID method in Manifest model #199

Closed

Conversation

willypuzzle
Copy link

Summary

Changed the way plugin ID is retrieved from the manifest. Instead of directly accessing the Id property, now using the GetPluginID() method for better encapsulation and consistency.

Ticket Link

Fixes mattermost/mattermost#26710

Related PR

mattermost/mattermost#27281

Changed the way plugin ID is retrieved from the manifest. Instead of directly accessing the Id property, now using the GetPluginID() method for better encapsulation and consistency.
@mattermost-build
Copy link
Contributor

Hello @willypuzzle,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Hi @willypuzzle, thanks for this contribution! We'll need to wait until mattermost/mattermost#27281 is merged, and then we'll update the imported version in this plugin's go.mod file. Just letting you know in case that wasn't known yet 👍 Nice work on this!

@willypuzzle
Copy link
Author

@mickmister sorry, if I bother you but but due that mattermost/mattermost#27281 was merged shouldn't we make start the Ci/CD again? Or I have to commit something?

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@willypuzzle
Copy link
Author

@mickmister we should merge this PR, it's relative to ne feature for API of the plugin

@mickmister
Copy link
Contributor

Hi @willypuzzle, thanks for pinging on this. There is one remaining issue in CI that needs to be addressed here https://github.com/mattermost/mattermost-plugin-starter-template/actions/runs/9380693770/job/25854231700?pr=199

# github.com/mattermost/mattermost-plugin-starter-template/build/manifest
Error: ./main.go:150:2[8](https://github.com/mattermost/mattermost-plugin-starter-template/actions/runs/9380693770/job/25854231700?pr=199#step:4:10):
manifest.GetPluginID undefined (type *model.Manifest has no field or method GetPluginID)
make: build/bin/manifest: No such file or directory
build/setup.mk:21: *** "Cannot parse id from ".  Stop.
Error: Process completed with exit code 2.

Specifically this line:

manifest.GetPluginID undefined (type *model.Manifest has no field or method GetPluginID)

In order for the plugin project to use this new method, the server dependency needs to be updated in the plugin project.

It's currently using v0.0.14:

github.com/mattermost/mattermost/server/public v0.0.14

The latest is v0.1.4 https://pkg.go.dev/github.com/mattermost/mattermost/server/public?tab=versions, though it doesn't contain your changes yet. We can reference a commit instead of a tag for our dependency version.


Can you try running this command to update the dependency? This is the merge commit from your PR mattermost/mattermost@4acc479

go get github.com/mattermost/mattermost/server/public@4acc4796edb2c1ff93e861b4732c1c758ac76371

Then you can run make or individual make commands defined here for testing.

The project's dependencies have been updated. This includes upgrading versions of several indirect dependencies and adding new ones. The changes also involve updating the version of the Mattermost server from v0.0.14 to v0.1.5-0.20240610151147-4acc4796edb2, among other updates.
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Jun 26, 2024
@hanzei hanzei added Invalid This doesn't seem right and removed 2: Dev Review Requires review by a core committer labels Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add plugin API to allow plugins to fetch their own pluginID
4 participants