Skip to content

Conversation

@armujahid
Copy link
Contributor

@armujahid armujahid commented Oct 17, 2019

What kind of change does this PR introduce?

Refactor /app/pages/Dashboard/index.js #2621 @Saeris @christianalfoni

What is the current behavior?

It was using cerebral for state management

What is the new behavior?

It uses Overmind for state management with help of useOvermind hook

What steps did you take to test this? This is required before we can merge.

  1. yarn lint

  2. yarn typecheck (Currently failing. @Saeris @christianalfoni kindly guide how to fix this type error)
    image

  3. yarn test

Checklist

client.resetStore();
}
useEffect(() => {
dashboard.dashboardMounted();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to fix this type error ? 🤔
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All CI checks (including typecheck) are green so It seems that this typecheck error was my environment specific.

Copy link
Contributor

Choose a reason for hiding this comment

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

May be an old thing that already got fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI typecheck is now failing because of this error after renaming the file to tsx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

@SaraVieira SaraVieira left a comment

Choose a reason for hiding this comment

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

Can you please rename the file to tsx?

@armujahid armujahid force-pushed the apppagesDashboardindex branch from 1cb881d to 0aa295b Compare October 19, 2019 20:30
@armujahid
Copy link
Contributor Author

@SaraVieira done

@lbogdan lbogdan temporarily deployed to pr2833 October 19, 2019 20:51 Inactive
@lbogdan lbogdan temporarily deployed to pr2833 October 19, 2019 21:31 Inactive
@lbogdan lbogdan temporarily deployed to pr2833 October 20, 2019 08:12 Inactive
@armujahid
Copy link
Contributor Author

All green again :)

Copy link
Contributor

@SaraVieira SaraVieira left a comment

Choose a reason for hiding this comment

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

Looks good!

Thank you!!

@SaraVieira SaraVieira merged commit d9ad79c into codesandbox:master Oct 20, 2019
@MichaelDeBoey MichaelDeBoey added 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧠 Overmind Indicates that this is related to the app's State Management 🔨 Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants