Skip to content

Conversation

@mkamranhamid
Copy link
Contributor

@mkamranhamid mkamranhamid commented Oct 6, 2019

…

What kind of change does this PR introduce?

I refactored /app/pages/Sandbox/Editor/Workspace/Dependencies/index.js and /app/pages/Sandbox/Editor/Workspace/Dependencies/index.js and when I run the app I was getting some errors so I had to update app/pages/Sandbox/Editor/Workspace/itemsNotOwnedSandboxInfo/NotOwnedSandboxInfo which was importing Dependencies as default export, changed it to import it as named export.
@Saeris and @christianalfoni please review it.

What is the current behavior?

Previously it was utilizing inject, hooksObserver and default export now we are using useOvermind with named export.
#2621

What is the new behavior?

Using useOvermind instead of inject and hooksObserver

What steps did you take to test this?

yarn lint
yarn test
Everything runs like a charm. It had some warning but not related to this PR

Checklist

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

I am not able to add 🔨 Refactor, 🧠 Overmind, and #Hacktoberfest labels. Can you please add them.

@vercel
Copy link

vercel bot commented Oct 6, 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-mkamranhamid-refactor-dependencies.codesandbox1.now.sh

@vercel vercel bot temporarily deployed to staging October 6, 2019 19:00 Inactive
@vercel vercel bot temporarily deployed to staging October 7, 2019 15:11 Inactive
@vercel vercel bot temporarily deployed to staging October 7, 2019 17:11 Inactive
@mkamranhamid
Copy link
Contributor Author

image
I run the yarn lint command and it worked like a charm without any errors
image
I guess the error occurred by version mismatch. I've changed the import order I guess the error was related to this because it says error `app/overmind` import should occur before import of `../../Dependencies` import/order

@vercel vercel bot temporarily deployed to staging October 7, 2019 21:13 Inactive
@SaraVieira
Copy link
Contributor

That was my merge error and fixed

Thank you!

@SaraVieira SaraVieira merged commit 261a71b into codesandbox:master Oct 7, 2019
@Saeris Saeris added Hacktoberfest 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Oct 8, 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.

3 participants