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

v8.1 Notice #350

Merged
merged 1 commit into from
Aug 25, 2023
Merged

v8.1 Notice #350

merged 1 commit into from
Aug 25, 2023

Conversation

amyblais
Copy link
Member

@amyblais amyblais commented Aug 24, 2023

Summary

  • In-product notice for v8.1 release.

Screenshots of the modals or screens in all target clients (required)

Test environment (required)

  • Server versions - v8.0 and v8.1

Test steps and expectation (required)

  • Spin up a v8.0 server - the notice should appear.
  • Spin up a v8.1 server - the notice should not appear.

@amyblais amyblais changed the title Update notices.json v8.1 Notice Aug 24, 2023
@amyblais amyblais added 1: PM Review Requires review by a product manager 2: QA Review Requires review by a QA tester labels Aug 24, 2023
Copy link
Contributor

@wiersgallak wiersgallak left a comment

Choose a reason for hiding this comment

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

Thanks, Amy!

@amyblais amyblais removed the 1: PM Review Requires review by a product manager label Aug 24, 2023
@lindalumitchell
Copy link
Contributor

@amyblais do you know if there is a way to apply these changes to test servers, specifying the different server versions? I'm not sure how to test this kind of change before it's merged. This repo can't spin up its own test servers I don't believe, even of just one version. 🤔

@amyblais
Copy link
Member Author

@lindalumitchell Here are instructions: https://github.com/mattermost/notices#how-to-test-notices-via-cloud-plugin.

@lindalumitchell
Copy link
Contributor

Thanks @amyblais! I'm afraid I'm still unclear about this first step from those instructions: "Prepare notices.json and merge it into the master branch." It doesn't appear that that has happened, so that I would be able to use /cloud and have these changes in effect, yeah? I didn't want to poke around with merging anything. I did spin up a v8.0 server just to double-check, but no notice appears.

@amyblais
Copy link
Member Author

amyblais commented Aug 25, 2023

@lindalumitchell The master branch is used only for testing purposes, so it is safe to merge there. Here is an example PR from Yasser #348 that he created to test this notice #346.

@amyblais
Copy link
Member Author

Since Yasser already has some experience with testing notices, I'm ok with waiting until next week and adding him as a reviewer here.

lindalumitchell added a commit that referenced this pull request Aug 25, 2023
@lindalumitchell
Copy link
Contributor

Ah I see, thank you Amy. I'm giving it a try, and if I don't get it we can wait until next week when Yasser is back.

Copy link
Contributor

@lindalumitchell lindalumitchell left a comment

Choose a reason for hiding this comment

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

Verified using #351 and #352 (per https://github.com/mattermost/notices#how-to-test-notices-via-cloud-plugin) and spinning up v8.0 and v8.1 servers using /cloud. Verified that the notice appears on v8.0 as expected and the links work, and that the notice does not appear on v8.1 as expected. 👍

@amyblais amyblais added 3: Reviews Complete All reviewers have approved the pull request and removed 2: QA Review Requires review by a QA tester labels Aug 25, 2023
@amyblais amyblais merged commit 3933a35 into release Aug 25, 2023
1 check passed
@amyblais amyblais deleted the amyblais-patch-2 branch August 25, 2023 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants