Skip to content

Conversation

@hardikmodi1
Copy link

What kind of change does this PR introduce?

This PR Refactors /app/pages/common/Modals/DeleteDeploymentModal/index.js to use Overmind

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

  1. Ran yarn test successfully
  2. Ran yarn lint successfully
  3. Ran yarn typecheck successfully
  4. Ran the application (yarn start) and confirmed the app started up ok

Checklist

  • Documentation
  • Testing
  • Ready to be merged

@vercel
Copy link

vercel bot commented Oct 14, 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/4wsle0c2a
🌍 Preview: https://codesandbox-clien-git-fork-hardiked-changedeletedeployme-9d602e.codesandbox1.now.sh

@vercel vercel bot temporarily deployed to staging October 14, 2019 19:25 Inactive
@CompuIves
Copy link
Member

Heyo, thanks for contributing! I think a change in the yarn.lock came with this PR, could you remove that change? Could you also test deleting a deployment? That way we are sure it still works.

@christianalfoni
Copy link
Contributor

@CompuIves this is part of Zeit deployment and not possible to test... what happened to our development token?

I have reviewed the code and the import, I think we should merge it in.

Should not really include  this file
Copy link
Contributor

@christianalfoni christianalfoni 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! 👍 😄

@MichaelDeBoey
Copy link
Contributor

This is a duplicate of #2809

@SaraVieira
Copy link
Contributor

Hey!

Thank you so much but seems this one has already been updated

:(

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.

7 participants