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

Fixes #28409 - Redux update changed snapshot #7229

Merged

Conversation

Ron-Lavi
Copy link
Member

@Ron-Lavi Ron-Lavi commented Dec 4, 2019

Because we merged the redux update and the middleware together we havn't notcie those errors.

Tested with the Interval Middleware and it still works as expected !

Because we merged the redux update and the middleware together we havn't notcie those errors.
@theforeman-bot
Copy link
Member

Issues: #28409

Copy link
Contributor

@sharvit sharvit left a comment

Choose a reason for hiding this comment

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

Thanks @LaViro LGTM 👍

@sharvit sharvit merged commit 4daf9b2 into theforeman:develop Dec 4, 2019
@mmoll
Copy link
Contributor

mmoll commented Dec 4, 2019

The tests of this PR were red. Also, travis tests on develop are broken now.

@sharvit
Copy link
Contributor

sharvit commented Dec 4, 2019

The tests of this PR were red. Also, travis tests on develop are broken now.

Tests were broken before because of 2 issues:

  1. https://projects.theforeman.org/issues/28409 (this PR fix this issue)
  2. https://projects.theforeman.org/issues/28391 - not that trivial and caused by react-bootstrap and patternfly-react

@mmoll
Copy link
Contributor

mmoll commented Dec 4, 2019

For Ruby gems, we would pin that package temporarily to fix the tests.

@sharvit
Copy link
Contributor

sharvit commented Dec 4, 2019

It is more complicated because react-bootstrap is a dependency of patternfly-react

@LaViro is investigating atm

@Ron-Lavi
Copy link
Member Author

Ron-Lavi commented Dec 4, 2019

@mmoll @sharvit, I opened issues to patternfly-react and react-bootstrap.
Patternfly said that they will investigate more about it this week and react-bootstrap didn't respond yet.

@sharvit
Copy link
Contributor

sharvit commented Dec 4, 2019

Can we pin pf in the vendor until then? Would it solve it?

@mturley
Copy link

mturley commented Dec 16, 2019

@sharvit this should be resolved in pf by patternfly/patternfly-react#3409

@sharvit
Copy link
Contributor

sharvit commented Dec 17, 2019

Thanks @mturley 👍

@Ron-Lavi Ron-Lavi deleted the fix/redux_update_middleware_snapshot branch February 12, 2020 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants