Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ghost_head cleanup & debug statements #8983

Merged
merged 3 commits into from
Sep 7, 2017

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Sep 7, 2017

This adds 3 cleanup / minor change commits on top of the error handling PR #8982.

  • Adds a handful of debug statements, so it's easier to see where time is spent after rendering when in DEBUG mode.
  • Changes the layout of promises to always have the . on a newline, which makes it easier to read
  • Changes from using Promise.props to Promise.join, which allows us to remove a few lines of code that otherwise don't do much

It would be possible to squash and merge this instead of #8982, setting the commit message back to the error handling one.

@kirrg001
Copy link
Contributor

kirrg001 commented Sep 7, 2017

I would squash these together and stick with the general commit message ghost_head cleanup & debug statements. The single commit messages are a bit too general e.g. Use join instead of props for less code - doesn't say where. If you want single commits, could you pls update the single messages? 🙃

@ErisDS
Copy link
Member Author

ErisDS commented Sep 7, 2017

It would be possible to squash and merge this instead of #8982, setting the commit message back to the error handling one.

This comment was if you merged this instead of #8982 as all 4 commits were here ;) Definitely squash this and give it a general message without an emoji!

@kirrg001 kirrg001 merged commit cdf6a10 into TryGhost:master Sep 7, 2017
@ErisDS ErisDS deleted the ghost_head_clean branch September 8, 2017 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants