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

Replaced legacy GitBook URLs with PL link #81

Closed
wants to merge 1 commit into from

Conversation

cwarnermm
Copy link
Contributor

Replaced absolute GitHub links with permalink via Marketing.

Addresses: https://mattermost.atlassian.net/browse/MM-55436

@cwarnermm cwarnermm added the 2: Dev Review Requires review by a core committer label Nov 10, 2023
@@ -2,7 +2,7 @@
"id": "com.mattermost.aws-sns",
"name": "AWS SNS",
"description": "Send alert notifications from Amazon AWS CloudWatch to Mattermost channels via AWS SNS.",
"homepage_url": "https://github.com/mattermost/mattermost-plugin-aws-SNS",
"homepage_url": "https://mattermost.com/pl/mattermost-plugin-aws-SNS",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we need to change this. The original URL is not a gitbook URL

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I'm thinking this just causes confusion if we change from pointing to the repo to somewhere else. I was under the impression the link would take me to the Mattermost docs site, but it's just a redirect to the repo's readme. As mentioned in the spreadsheet linked in the PR description:

In Yoast because of hash - but since it's an external link doesn't really matter

It seems that it "doesn't matter" if we make this change or not, so I propose we keep it how it is to avoid confusion

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@cwarnermm cwarnermm Nov 20, 2023

Choose a reason for hiding this comment

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

Long term, yes. Mattermost-supported plugin README content will likely move back to the product docs site, though a timeline for that work hasn't been set yet. However, that's not driving this change primarily.

These changes were applied to bring Mattermost-supported content in line with our Handbook policy of linking to documentation via permalinks managed by Marketing.

I applied these changes to the in-product text visible via the System Console.

If this change will introduce confusion in the short term, I'm 0/5 on keeping these proposed changes and open to reverting to the GitHub links instead. We could decide to address links if/when we decide to move content in the future.

Copy link
Contributor

@mickmister mickmister Nov 21, 2023

Choose a reason for hiding this comment

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

If this change will introduce confusion in the short term, I'm 0/5 on keeping these proposed changes and open to reverting to the GitHub links instead. We could decide to address links if/when we decide to move content in the future.

Thanks for the explanation @cwarnermm 👍

Yes I think I would prefer this for now. @hanzei Thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm 1/5 that we don't need perma links here. We can always release a new plugin version if we really want to change a link. I don't think it's worth the confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Thank you both for your feedback and input. I'll close this PR and the other PRs introducing the same updates, as @mickmister listed above.

@cwarnermm
Copy link
Contributor Author

Holding off on applying this change for the time being based on internal feedback. Closing.

@cwarnermm cwarnermm closed this Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants