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

Migrate BrowserStore to redux #241

Merged
merged 1 commit into from
Nov 7, 2017
Merged

Migrate BrowserStore to redux #241

merged 1 commit into from
Nov 7, 2017

Conversation

jespino
Copy link
Member

@jespino jespino commented Nov 4, 2017 •
edited
Loading

Related to the Github Issue: mattermost/mattermost#7236

Checklist

  • Added or updated unit tests (required for all new features)
  • Has redux changes (please link)

@jasonblais jasonblais added the 2: Dev Review Requires review by a core commiter label Nov 4, 2017
@jespino jespino force-pushed the migrate-browser-store branch 2 times, most recently from cf1e182 to c28b194 Compare November 5, 2017 11:49
jwilander
jwilander reviewed Nov 7, 2017
View reviewed changes
Copy link
Member

@jwilander jwilander left a comment

Choose a reason for hiding this comment

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

Awesome work on this @jespino ! This is very much what I envisioned for this

LGTM other than a minor comment on package.json

package.json Outdated
@@ -23,6 +23,7 @@
"jquery": "3.2.1",
"key-mirror": "1.0.1",
"localforage": "1.5.0",
"localforage-observable": "^1.4.0",
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove the ^ so we have to explicitly update?

package.json Outdated
@@ -94,6 +95,7 @@
"raw-loader": "0.5.1",
"react-addons-test-utils": "15.6.2",
"react-test-renderer": "15",
"redux-persist-node-storage": "^1.0.2",
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@jespino
Copy link
Member Author

jespino commented Nov 7, 2017

Fixed the packages.json and rebased to master :)

hmhealey
hmhealey approved these changes Nov 7, 2017
View reviewed changes
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Nice work! Looks good to me

@jwilander jwilander added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Nov 7, 2017
@jwilander jwilander merged commit 399de2b into mattermost:master Nov 7, 2017
@lindalumitchell lindalumitchell added the Tests/Not Needed Does not require new release tests label Nov 14, 2017
@esethna esethna added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Dec 4, 2017
hmhealey pushed a commit that referenced this pull request Aug 28, 2020
@saturninoabril @hmhealey
filter out punctuations in sorting channel display name (#241)
131a559
hmhealey pushed a commit that referenced this pull request Mar 17, 2021
@saturninoabril @enahum
filter out punctuations in sorting channel display name (#241)
b9095ae
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jwilander jwilander jwilander approved these changes

@hmhealey hmhealey hmhealey approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jespino @jwilander @hmhealey @jasonblais @esethna @lindalumitchell