[MM-53695] Make use of github.com/mattermost/mattermost/server/public

[hanzei]

Summary

This PR makes use of the newly created go module in https://pkg.go.dev/github.com/mattermost/mattermost/server/public.

I intentionally did updatemin_server_version as the plugin does still work with v7 servers.

Once https://github.com/mattermost/mattermost-plugin-api gets moved to the mono repo, I will update the plugin to make use of the pluginapi.Client and advocate for its usage over p.API.

I also snuck a go update to 1.19 into the PR. This gets the plugin in sync with the go version used in other plugins. CI is already updated.

Ticket Link

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

[codecov-commenter]

Codecov Report

Patch and project coverage have no change.

Comparison is base (b874f47) 5.26% compared to head (e12ec05) 5.26%.

Additional details and impacted files

@@          Coverage Diff           @@
##           master    #185   +/-   ##
======================================
  Coverage    5.26%   5.26%           
======================================
  Files           3       3           
  Lines          38      38           
======================================
  Hits            2       2           
  Misses         36      36           

Impacted Files	Coverage Δ
server/main.go	0.00% <ø> (ø)
server/plugin.go	100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[moussetc]

Question as a plugin developper: previously to this change, it was easy to know the compatibility between plugin and server version (to know which minimum version to put in the plugin.json file).
With the new package, which seems to have its own independent versioning scheme, where can we find the compatibility table?

[lieut-data]

Thanks for highlighting this, @moussetc! @amyblais / @agnivade, I'm wondering we should plan to tag this submodule at least once a release (ideally at the same commit as the release?), and include that with the release notes.

FWIW, in practice, I think you should be able to use the latest public submodule at all times (within the same major version, at least), it's just that not all methods will be supported by the server -- just like today. We do plan to make semantically breaking changes before we hit v1, but only to things we don't expect public consumption of (e.g. our plugin internals that will get refactored back into the core and don't affect compilation or runtime semantics).

[amyblais]

@agnivade Would you like me to add this to the same ticket (e.g. with https://mattermost.atlassian.net/browse/MM-53676) moving forward? What would an example release note look like?

[moussetc]

Thanks @lieut-data ! Giving the recommended module version in the server's release notes is perfect :)

[agnivade]

@amyblais - This is different from bumping dependencies. If we tag a new release for every main release, then there is no need to add a new tag every time we bump the dependencies because anyways it's going to be bumped.

I think this should be part of the task which tags the main release. And lastly, if there are no new commits from the last release, we should not be unnecessarily bumping up the tag of course.

[amyblais]

Ok, I've opened a ticket to add this to the release tagging process https://mattermost.atlassian.net/browse/CLD-6013.

[mickmister]

@moussetc I want to point out that the version of mattermost-server imported in go.mod does not directly coincide with the required min_server_version in the plugin's plugin.json file. Typically, the available plugin RPC API and hooks needed for the plugin are the actual server requirements. So the min_server_version should generally be governed by the use of API and hooks. This way, we can have the min_server_version be applicable to more servers running slightly earlier server versions than what is imported.

[mickmister]

LGTM

[lieut-data]

I wonder if we should abandon the nested build/go.mod and just rely on the top-level one? I don't think we need to manage versions independently, and Go will already only compile what it actually needs.

[mickmister]

@lieut-data Personally, I like the build/go.mod file being separate, because there are dependencies needed there that aren't needed for the source code of a given plugin. And although those dependencies are not included in the compiled plugin, having unrelated dependencies in the top-level go.mod file can add noise to what the given plugin is "all about".

There is also somewhat essentially dead code in the build folder, more specifically build/sync, which is using the dependency github.com/go-git/go-git/v5. If we were move the build/go.mod dependencies into go.mod, then I would also suggest to remove this build/sync folder to remove that dependency.

Saying this out loud is making me think we should probably remove that folder regardless of the build/go.mod decision. It was an attempt at having synchronization logic between plugin repos #86, but it unfortunately didn't end up being used in our tooling. Not part of this PR, though wondering thoughts on removing this as a means of code cleanup.

[lieut-data]

👍 on removing build/sync. 0/5 on consolidating go.mod -- happy to defer to your thinking above :)