search

mattermost / mattermost-plugin-starter-template

Build scripts and templates for writing Mattermost plugins.
https://developers.mattermost.com/extend/plugins/
Apache License 2.0
129 stars 120 forks source link

Removed trailing slash from site url. #120

Closed shieldsjared closed 3 years ago

shieldsjared commented 4 years ago

Summary

If the MM_SERVICESETTINGS_SITEURL environment variable is set with a trailing slash, the API calls made to upload the plugin result in unpredictable results. Code added simply removes any trailing slash included in the env variable.

Probably could have migrated all of the siteURL related code to get/validate/trim the siteURL into a function, but not sure it was really necessary for the overall scope of pluginctl.

Ticket Link

Fixes issue described in #115.

mattermod commented 4 years ago

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!

/cc @jasonblais @jfrerich

shieldsjared commented 4 years ago

phew finally got a moment to submit that simple PR:

https://github.com/mattermost/mattermost-server/pull/15513

lieut-data commented 4 years ago

Awesome, thanks @shieldsjared :)

mattermod commented 3 years ago

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!

/cc @jasonblais @jfrerich

jasonblais commented 3 years ago

@shieldsjared Quick update (which you may have already seen): https://github.com/mattermost/mattermost-server/pull/15513 is merged so the remaining step is to

update the go module dependency to point at the new master commit. The go.mod looks weird because it's referencing a pseudo-version from the master branch vs. a tagged release -- namely commit ed34468996e6 (from around July 23rd).

No rush by the way! I'll remove the Lifecycle/1:stale label since I know this was recently been worked on (via the mattermost-server PR).

shieldsjared commented 3 years ago

@jasonblais - Updated the PR to remove the internal fix for the trailing slash, and instead pointing to the specific commit e726b0426826 in mattermost-server that contains the fix w/in the client. Ready for a fresh review :)

jasonblais commented 3 years ago

@hanzei @lieut-data ready for a fresh review

lieut-data commented 3 years ago

Skipping QA review since already tested in the upstream server change.

lieut-data commented 3 years ago

Thanks so much, @shieldsjared!