-
Notifications
You must be signed in to change notification settings - Fork 632
Conversation
nadavsinai
commented
Aug 10, 2016
- update dependencies
- add appModule,
- fix bootstrap
- remove boilerplate from inner components
- update dependencies - add appModule, - fix bootstrap - remove boilerplate from inner components
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
I offer to make another PR where github route will be pulled to its own module, with router async loading of this module and child routes, however since this is webpack based and router has dependency on SystemJS loader it will require upgrading to webpack 2 (2.1.beta.20, and adding a runtime of System polyfill (System.js), anyone intrested in this? |
@@ -22,19 +22,23 @@ | |||
"raw-loader": "^0.5.1", | |||
"to-string-loader": "^1.1.4", | |||
"typescript": "~1.8.9", | |||
"typings": "~1.0.3", | |||
"typings": "~1.3.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a mandatory change to update to RC5? If not please revert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not mandatory, just a updated version for typings, I thought it'll be a best practice to keep things up to date, will revert
@nadavsinai thnx for the PR! I would gladly merge it but there are number of un-related changes in there and it is hard to see what is strictly needed for RC5. Could you please revert all the non-essential parts? Also, please don't change naming / formatting in this PR - we can discuss those in a separate issue / PR. Would you be willing to make the above changes so I can merge this one? |
This looks good now! I will squash while merging. Thnx! |