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

[MM-15039] Revert react-redux to 5.1.1 fixing the issue mostly on profile pop-over and edit channel header components #2636

Merged
merged 1 commit into from
Apr 10, 2019

Conversation

saturninoabril
Copy link
Member

Summary

Seems to be related to recent update of react-redux to v6.0.1 which might not be compatible with react-bootstrap@0.32.4 (per react-redux discussion).

I'd suggest to upgrade react-redux to v6/v7 once react-bootstrap releases the stable version of v1.

Ticket Link

Jira - MM-15039

@saturninoabril saturninoabril added the 2: Dev Review Requires review by a core commiter label Apr 10, 2019
@enahum
Copy link
Contributor

enahum commented Apr 10, 2019

Saturnino why not upgrading the react-bootstrap library instead?

@saturninoabril
Copy link
Member Author

saturninoabril commented Apr 10, 2019

I did try but it's giving me another set of errors. Besides, it's still in beta - 1.0.0-beta.6. I don't know if we want that. The last stable version is the one we have right now - 0.32.4.

@enahum
Copy link
Contributor

enahum commented Apr 10, 2019

I think we should move fwd instead of backwards. Having said that I’ll approve just because of the beta version BUT I want to ensure that there is a follow up ticket for this.

@saturninoabril
Copy link
Member Author

@enahum I agree. Ticket filed - https://mattermost.atlassian.net/browse/MM-15040

@crspeller
Copy link
Member

@saturninoabril @enahum That's going to be quite the ticket.
Looks like react-bootstrap upgraded to bootstrap 4 for v1: https://react-bootstrap.github.io/migrating/

You might want to consider fixing this now rather than relying on that upgrade.

@enahum
Copy link
Contributor

enahum commented Apr 10, 2019

@crspeller I’m with you but in any case I think we should downgrade the react-redux vs as is breaking current functionality and when upgrading again we should make sure everything works, there is no point in upgrading a dependency if we are not addressing the breaking changes.

Then after the issues are fixed we could work on it.

Copy link
Member

@crspeller crspeller left a comment

Choose a reason for hiding this comment

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

Sorry. I meant to approve. My comments are not blocking.

@saturninoabril
Copy link
Member Author

Thanks @crspeller @enahum. I added that comment to https://mattermost.atlassian.net/browse/MM-15040

@saturninoabril saturninoabril merged commit 58f6747 into mattermost:master Apr 10, 2019
@saturninoabril saturninoabril deleted the revert-react-redux branch April 10, 2019 15:30
@lindy65 lindy65 added Tests/Not Needed Does not require new release tests and removed 2: Dev Review Requires review by a core commiter labels Apr 13, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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
Development

Successfully merging this pull request may close these issues.

5 participants