Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-36906 Remove timedDND feature flag and fix e2e tests #8824

Merged
merged 10 commits into from
Oct 13, 2021
Merged

MM-36906 Remove timedDND feature flag and fix e2e tests #8824

merged 10 commits into from
Oct 13, 2021

Conversation

darkLord19
Copy link
Contributor

@darkLord19 darkLord19 commented Sep 9, 2021 •
edited by hmhealey
Loading

Ticket Link

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

Release Note

Remove feature flag for Timed DND feature

@mm-cloud-bot
Copy link

@darkLord19: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@mattermod
Copy link
Contributor

Hello @darkLord19,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@mattermod mattermod added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Sep 9, 2021
@darkLord19 darkLord19 mentioned this pull request Sep 9, 2021
Merged
@jgilliam17 jgilliam17 requested review from devinbinnie, hmhealey, Willyfrog and ashishbhate and removed request for hmhealey and devinbinnie September 9, 2021 21:01
@jgilliam17
Copy link
Contributor

jgilliam17 commented Sep 9, 2021 •
edited
Loading

Sorry. Changing reviewers, I picked the wrong team first time around.

@Willyfrog
Copy link
Contributor

Hello @darkLord19 and thanks for the contribution, is there a ticket or somewhere where I can read the expected changes?

@darkLord19
Copy link
Contributor Author

darkLord19 commented Sep 13, 2021 •
edited
Loading

@Willyfrog https://mattermost.atlassian.net/browse/MM-36906 this is the ticket for this pr and this is the ticket for feature https://mattermost.atlassian.net/browse/MM-8497

@hmhealey hmhealey changed the title remove timedDND feature flag and fix e2e tests MM-36906 Remove timedDND feature flag and fix e2e tests Sep 14, 2021
@hmhealey
Copy link
Member

Added the ticket number to the title and description so that the Jira integration picks everything up correctly

@Willyfrog Willyfrog added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Sep 15, 2021
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Sep 15, 2021
Willyfrog
Willyfrog approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@Willyfrog Willyfrog left a comment •
edited
Loading

Choose a reason for hiding this comment

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

test servers seem to be having trouble at the moment, but code looks good

@darkLord19 can you fix the conflict?

@jgilliam17
Copy link
Contributor

Gentle ping to review @ashishbhate @saturninoabril I'd like to see if we can get this in before code complete for v6.0 tomorrow. Thanks 🙂

@esethna
Copy link
Contributor

esethna commented Sep 29, 2021 •
edited
Loading

@darkLord19 agree, feel free to submit that in a separate PR if you'd like.

@saturninoabril this PR should just be scoped to the removal of the feature flag

saturninoabril
saturninoabril approved these changes Sep 29, 2021
View reviewed changes
Copy link
Member

@saturninoabril saturninoabril left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@esethna I understand. In my mind, "removal of the feature flag" is acceptance of a feature into the core after passing our quality standard.

Anyway, I've filed a ticket to address it separately - https://mattermost.atlassian.net/browse/MM-38937.

@darkLord19 Thank you so much for your contribution!

@esethna
Copy link
Contributor

esethna commented Sep 29, 2021

@saturninoabril yes you're totally right, so super appreciate you filing the bug. It's not a blocker in my mind to removing the flag though :)

@jgilliam17
Copy link
Contributor

/update-branch

@mattermod
Copy link
Contributor

Error trying to update the PR.
Please do it manually.

@mattermod mattermod mentioned this pull request Oct 4, 2021
Closed
@jgilliam17
Copy link
Contributor

@darkLord19 Can you please update the PR and fix merge conflicts? Thanks

@darkLord19
Copy link
Contributor Author

@jgilliam17 fixed conflicts and e2e tests

@saturninoabril
Copy link
Member

@darkLord19 Thanks for updating!

@jgilliam17 Here is the E2E - https://mm-cypress-report.s3.amazonaws.com/49169-4501b3e-ee-ent-webapp-pr-8824-mattermost-enterprise-edition:4501b3e/mochawesome.html (All good, failed tests were known issues and not related to this PR)

@esethna
Copy link
Contributor

esethna commented Oct 12, 2021

@jgilliam17 is this good to merge now?

@esethna esethna removed the 2: Dev Review Requires review by a core commiter label Oct 12, 2021
@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 13, 2021
@jgilliam17
Copy link
Contributor

/update-branch

@jgilliam17
Copy link
Contributor

jgilliam17 commented Oct 13, 2021 •
edited
Loading

Thanks @darkLord19
There is small issue with check mark, help text and arrow alignment, not reproducing on master.
Screen Shot 2021-10-13 at 10 36 03 AM

@darkLord19
Copy link
Contributor Author

@jgilliam17 there was some minor css typo issue. Updated pr.

@jgilliam17
Copy link
Contributor

jgilliam17 commented Oct 13, 2021 •
edited
Loading

@darkLord19 can you please check one more thing. I am not seeing the fix for full submenu item width on the server
re: #8996
Can you make sure item is taking the whole width? Thanks
Screen Shot 2021-10-13 at 1 50 48 PM

@darkLord19
Copy link
Contributor Author

@jgilliam17 integrated the mentioned change

jgilliam17
jgilliam17 approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Thank you @darkLord19
Tested, looks good to merge.

@jgilliam17 jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Oct 13, 2021
@mm-cloud-bot
Copy link

Test server destroyed

@jgilliam17 jgilliam17 merged commit 28f34d4 into mattermost:master Oct 13, 2021
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jgilliam17 jgilliam17 jgilliam17 approved these changes

@saturninoabril saturninoabril saturninoabril approved these changes

@ashishbhate ashishbhate ashishbhate approved these changes

@Willyfrog Willyfrog Willyfrog approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation release-note
Projects
None yet
Milestone
v6.1.0
Development

Successfully merging this pull request may close these issues.
10 participants
@darkLord19 @mm-cloud-bot @mattermod @jgilliam17 @Willyfrog @hmhealey @saturninoabril @esethna @ashishbhate @amyblais