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

Jump to bottom

[MM-15305] Migrate Preference.CleanupFlagsBatch to Sync by default #10858

Merged
merged 4 commits into from May 24, 2019
Merged

[MM-15305] Migrate Preference.CleanupFlagsBatch to Sync by default #10858

merged 4 commits into from May 24, 2019

Conversation

farhadab
Copy link
Contributor

Summary

This PR migrates the CleanupFlagsBatch method from the Preference store to Sync by
default.

Ticket Link

Fixes #10712

@hanzei hanzei requested review from jespino and hanzei May 16, 2019 19:06
@hanzei hanzei added the 2: Dev Review Requires review by a developer label May 16, 2019
@hanzei
Copy link
Contributor

hanzei commented May 16, 2019

@jespino The tests fail because this breaks the EE code. Is there a way to run the TE tests anyway?

hanzei
hanzei reviewed May 16, 2019
View reviewed changes
Copy link
Contributor

@hanzei hanzei 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 for this PR @farhadab.

I leaved a few comments.

store/sqlstore/preference_store.go Outdated Show resolved Hide resolved
store/sqlstore/preference_store.go Outdated Show resolved Hide resolved
store/sqlstore/preference_store.go Outdated Show resolved Hide resolved
hanzei
hanzei approved these changes May 17, 2019
View reviewed changes
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

LGTM 👍

jespino
jespino approved these changes May 17, 2019
View reviewed changes
Copy link
Member

@jespino jespino left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks! Great work @farhadab! ✨

I going to try to make the tests pass changing thing in the enterprise repo.

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request Do Not Merge Should not be merged until this label is removed and removed 2: Dev Review Requires review by a developer labels May 17, 2019
@jespino
Copy link
Member

jespino commented May 22, 2019

@farhadab Now the entepreprise repo changes are approved, do you mind to merge master into the branch?

@jespino jespino merged commit 6158adb into mattermost:master May 24, 2019
@hanzei hanzei removed the Do Not Merge Should not be merged until this label is removed label May 24, 2019
@hanzei hanzei added this to the v5.14.0 milestone May 24, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels May 24, 2019
@lindy65 lindy65 added Tests/Not Needed New release tests not required and removed 4: Reviews Complete All reviewers have approved the pull request labels Jun 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jespino jespino jespino approved these changes

@hanzei hanzei hanzei approved these changes

Assignees

@jespino jespino

Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed New release tests not required
Projects
None yet
Milestone
v5.14.0
Development

Successfully merging this pull request may close these issues.

[MM-15305] Migrate "Preference.CleanupFlagsBatch" to Sync by default
5 participants
@farhadab @hanzei @jespino @lindy65 @amyblais