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

Include stubs for all hooks to get going quickly #110

Open
mickmister opened this issue Jul 29, 2020 · 6 comments
Open

Include stubs for all hooks to get going quickly #110

mickmister opened this issue Jul 29, 2020 · 6 comments
Labels
Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Hacktoberfest Help Wanted Community help wanted Tech/Go Type/Enhancement New feature or improvement of existing feature Up For Grabs Ready for help from the community. Removed when someone volunteers

Comments

@mickmister
Copy link
Contributor

Original Message

If https://github.com/mattermost/mattermost-plugin-starter-template would contain a stub implementation of all hooks, new author could use it from the get go and even if they leave the unneeded hooks, it's not a big deal.

Issue created from a Mattermost message by @hanzei.

@mickmister mickmister added Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Help Wanted Community help wanted Tech/Go Type/Enhancement New feature or improvement of existing feature Up For Grabs Ready for help from the community. Removed when someone volunteers labels Jul 29, 2020
@mickmister
Copy link
Contributor Author

This would be a great learning experience for someone to learn about the different hooks available in depth.

@lieut-data
Copy link
Member

Note that this has a non-zero impact on performance if a user mistakenly leaves these stubs in place: the server optimizes away the need to notify plugins of hooks that it doesn't register.

@mickmister
Copy link
Contributor Author

@lieut-data I was thinking the same. Do you have any ideas of how we can help a plugin dev new to the framework have some sort of IDE support for the hooks? Linking to the docs does our due diligence, but there are so many functions that having autocomplete like p.API, but for hooks, would be immensely helpful. I am not aware of a way to do this with Go for creating methods on a struct. Please see the discussion at https://community-daily.mattermost.com/_redirect/pl/3oyebhq9w7b9bmzmbemz5573dh for context.

@lieut-data
Copy link
Member

I'm not particularly fond of the "implement the method" magic that currently exists. I'd much rather have something like a p.API.RegisterExecuteCommand(p.ExecuteCommand) (or p.Hooks.RegisterExecuteCommand(p.ExecuteCommand)).

In principle, this is possible, but it requires reworking the implementation of hooksRPCServer to support same, looking to a registry defined at run-time instead of reflecting. It's a bunch of fiddly work that might duplicate code (unless we relocated all of it to the mattermost-plugin-api and had the server depend on /that/). But at the end of the day, we get auto-completion :D

@mickmister
Copy link
Contributor Author

unless we relocated all of it to the mattermost-plugin-api and had the server depend on /that/

@lieut-data That would just be another place that needs to be updated whenever a new hook is implemented :) Our docs are a bit out of date in that regard and I have a todo to update those. I suppose we could have the stubs and comment them out?

@lieut-data
Copy link
Member

That would just be another place that needs to be updated whenever a new hook is implemented :)

So in my proposal, that would be the /definition/ of the hooks, and to implement same in the server, we'd bump our dependency on mattermost-plugin-api and implement the required interface. In theory, at least, no duplication required at all.

I suppose we could have the stubs and comment them out?

This is pragmatic: I like it! (Same duplication problem, of course.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Hacktoberfest Help Wanted Community help wanted Tech/Go Type/Enhancement New feature or improvement of existing feature Up For Grabs Ready for help from the community. Removed when someone volunteers
Projects
None yet
Development

No branches or pull requests

4 participants