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

Angular2 chat app #609

Open
wants to merge 11 commits into
base: next
Choose a base branch
from
Open

Angular2 chat app #609

wants to merge 11 commits into from

Conversation

deadcee
Copy link

@deadcee deadcee commented Jun 20, 2016

This is a similar pull request to the one summited by @zubair-io.
Includes some fixes to the index.html


This change is Reviewable

@zubair-io
Copy link
Contributor

Hi @deadcee, thanks for the changes
In your commit 7663c68 you checkin the .map, .js and typings files. I would not check them in since they a built at runtime or installed during the setup

@den-t den-t force-pushed the angular2-chat-app branch from 862a1cc to e32db0d Compare June 21, 2016 02:16
@deadcee
Copy link
Author

deadcee commented Jun 21, 2016

The offending commit has been removed. Thanks for pointing it out.

@dalanmiller
Copy link
Contributor

dalanmiller commented Jun 22, 2016

Awesome demo @deadcee!

I'm going to say that we probably don't want to commit the .hz/config.toml file. We've been pretty much resolute about this just because of possible security issues. @deontologician?

Once I get back in the office let me spin it up and then we can get this merged 👍

@den-t den-t force-pushed the angular2-chat-app branch from e32db0d to b146e55 Compare June 22, 2016 22:53
@deadcee
Copy link
Author

deadcee commented Jun 22, 2016

config.toml was removed and better user guidelines in README.md

@zubair-io
Copy link
Contributor

@deadcee you do not need to run step 3 and in step 4 tsc.
Typescript is installed in the package.json and the typescript are compiled in browser at run time 😀

@dalanmiller
Copy link
Contributor

Hey @deadcee would you mind taking a look at this again when you have a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants