Skip to content

Conversation

@MichaelDeBoey
Copy link
Contributor

Follow-up of #2727

Things I did extra:

  • Cleanup useEffect call
  • Export NetlifyLogs by a named export instead of a default export

@MichaelDeBoey MichaelDeBoey added 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Nov 22, 2019
@lbogdan lbogdan temporarily deployed to pr3068 November 22, 2019 13:47 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Nov 22, 2019

Build for latest commit 534893e is at https://pr3068.build.csb.dev/s/new.

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.

It looks great! I just have a couple of improvements on UX 😄

Tested:

  • Deploy to Netlify
  • Open logs
  • With the changes suggested it now stays in "Waiting for build to start..." until it actually gets some logs
  • Close log, and open again

Just merge suggestions and this is ready 😄

@lbogdan lbogdan temporarily deployed to pr3068 November 28, 2019 17:35 Inactive
@lbogdan lbogdan temporarily deployed to pr3068 December 1, 2019 00:22 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the overmind/NetlifyLogs branch 3 times, most recently from 3da1a3f to 18b7556 Compare December 4, 2019 17:50
@lbogdan lbogdan temporarily deployed to pr3068 December 4, 2019 18:17 Inactive
@SaraVieira
Copy link
Contributor

awesome! works great!!

@SaraVieira SaraVieira merged commit 169c8e8 into codesandbox:master Dec 8, 2019
@MichaelDeBoey
Copy link
Contributor Author

With pleasure 🙂

@MichaelDeBoey MichaelDeBoey deleted the overmind/NetlifyLogs branch December 8, 2019 17:42
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