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

Adding coral embed stream #4

Merged
merged 7 commits into from
Nov 2, 2016
Merged

Adding coral embed stream #4

merged 7 commits into from
Nov 2, 2016

Conversation

davidgljay
Copy link

@davidgljay davidgljay commented Nov 1, 2016

What does this PR do?

Adds coral-embed-stream to the newly structured talk repo.

How do I test this PR?

  1. npm install
  2. From the repo root, run npm run embed-start
  3. Dev server should start.
  4. Comment stream should load with mock data.
  5. Posts won’t work, and a few 404s are expected until we get the endpoints up.

Notes

  • I’ve left the original web pack in place for reference, I’m not certain what the vision is for a central webpack so wanted to keep that separate. If run independently, this webpack file will build to the target dist directory.

@riley
Copy link

riley commented Nov 1, 2016

I'm getting this error @davidgljay

screen shot 2016-11-01 at 4 34 13 pm

@davidgljay
Copy link
Author

@riley This issue should be fixed. Also, I removed Dynamic Containers since there's no longer a user case for them and replaced with vanilla react.

Note: CommentStream is now has a gigantic render function, and under most circumstances it would be worth breaking up. I've avoided doing so

  1. To limit the scope of the refactor to what's absolutely necessary.
  2. Because it's likely that there will be benefits to having one giant render function down the road, and there's no downside other than unfashionableness.

Copy link

@riley riley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! 💯

@riley riley merged commit 0facdbb into master Nov 2, 2016
@riley riley deleted the add-coral-embed-stream branch November 2, 2016 16:33
@riley riley mentioned this pull request Nov 14, 2016
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.

3 participants