Skip to content

Conversation

@stackyism
Copy link
Contributor

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?

/Sandbox/Editor/Workspace/items/Live/index.js was using inject and observer from app/componentConnectors

What is the new behavior?

uses useOvermind hook from app/overmind

What steps did you take to test this?

Opened codesandbox on local with my logged-in user by replacing jwt token.
And while logged in tested the successful rendering of LiveInfo Component inside Live/index.js.
Successfully ran yarn typecheck, yarn lint and yarn test.

Checklist

  • Documentation:N/A
  • Testing
  • Ready to be merged
  • Added myself to contributors table: N/A

@vercel
Copy link

vercel bot commented Oct 5, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/codesandbox/codesandbox-client/9ii1eclll
🌍 Preview: https://codesandbox-cl-git-fork-stackyism-refactor-sandbox-edito-7873e4.codesandbox1.now.sh

@vercel vercel bot temporarily deployed to staging October 5, 2019 18:59 Inactive
@stackyism stackyism force-pushed the refactor-sandbox-editor-workspace-live-tsx branch from 4d3b090 to 4114741 Compare October 5, 2019 21:14
@vercel vercel bot temporarily deployed to staging October 5, 2019 21:15 Inactive
@Saeris Saeris added Hacktoberfest 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Oct 8, 2019
@stackyism stackyism force-pushed the refactor-sandbox-editor-workspace-live-tsx branch from 4114741 to cdab9e4 Compare October 11, 2019 21:31
@vercel vercel bot temporarily deployed to staging October 11, 2019 21:31 Inactive
@stackyism
Copy link
Contributor Author

Hello @Saeris & @SaraVieira .
I have rebased my branch with the latest master which fixed the failing integration test.
Would you please suggest any further action required from my side for getting this PR merged?

Thank you so much for providing this opportunity to contribute. :)

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! All checks are passing too so I think this can be merged. Thanks!

@Saeris Saeris merged commit 6264ce7 into codesandbox:master Oct 17, 2019
@stackyism
Copy link
Contributor Author

Looks good! All checks are passing too so I think this can be merged. Thanks!

Thank you so much @Saeris ! :)

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.

2 participants