Skip to content

Conversation

@ilhamwahabi
Copy link
Contributor

@ilhamwahabi ilhamwahabi commented Oct 4, 2019

What kind of change does this PR introduce?

Refactors code as a part of hacktoberfest mentioned in #2621.
@Saeris @christianalfoni

What is the current behavior?

packages/app/src/app/pages/CliInstructions/index.tsx was using inject and hooksObserver from app/componentConnectors

What is the new behavior?

uses useOvermind from app/overmind

What steps did you take to test this?

run yarn test, yarn typecheck and through lint-staged also tried to run the app with yarn start

Checklist

  • Documentation
  • Testing
  • Ready to be merged
  • Added myself to contributors table

@vercel
Copy link

vercel bot commented Oct 4, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://codesandbox-client-git-fork-iwgx-refactor-cliinstructions-index.codesandbox1.now.sh

@vercel vercel bot temporarily deployed to staging October 4, 2019 18:51 Inactive
@ilhamwahabi ilhamwahabi changed the title ref: CliInstructions - replace Cerebral with Overmind 🔨 Refactored 🧠 Overmind Hacktober | /app/pages/CliInstructions/index.tsx : refactor to replace Cerebral with Overmind Oct 4, 2019
@ilhamwahabi ilhamwahabi force-pushed the refactor-cliinstructions-index branch from dc87ba2 to 6e7b07d Compare October 4, 2019 19:13
@vercel vercel bot temporarily deployed to staging October 4, 2019 19:13 Inactive
@vercel vercel bot temporarily deployed to staging October 4, 2019 19:40 Inactive
@ilhamwahabi ilhamwahabi force-pushed the refactor-cliinstructions-index branch from 9792e41 to 6e7b07d Compare October 4, 2019 19:53
@Saeris Saeris added Hacktoberfest 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Oct 8, 2019
Copy link
Contributor

@Saeris Saeris 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 to me, just some minor changes to imports I'd like you to fix and then this should be good to go!

@ilhamwahabi
Copy link
Contributor Author

thanks for your review @Saeris
you can check it again

@lbogdan lbogdan temporarily deployed to pr2656 October 15, 2019 15:51 Inactive
@CompuIves
Copy link
Member

Heyo! I tested this PR and it seems like it doesn't work, when going to /s/cli I just see a CodeSandbox Logo. The fix is changing the import of CliInstruction to be const { CliInstruction } = ... rather than default import since you've changed that.

Copy link
Member

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

Need to change default import.

@ilhamwahabi
Copy link
Contributor Author

ilhamwahabi commented Oct 17, 2019

Sorry for that, can you check again my latest update? Thanks! @CompuIves
It should be worked now

image

@CompuIves
Copy link
Member

Very nice! Thanks a lot!

@CompuIves CompuIves merged commit f51b052 into codesandbox:master Oct 24, 2019
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