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

Upgrade to angular 2.0.0-rc.1 #176

Merged
merged 6 commits into from
May 7, 2016
Merged

Conversation

ciriarte
Copy link
Contributor

@ciriarte ciriarte commented May 3, 2016

Hi Guys,

I've updated the code to point to 2.0.0-rc.1, please let me know if it looks OK or you want me to change anything.

@ciriarte
Copy link
Contributor Author

ciriarte commented May 6, 2016

@valorkin sorry to bother you again! I've updated this one as well to rc.1 and made Travis happy-green. Could you please take a look whenever you have the chance? Good luck at ngconf!

@ciriarte
Copy link
Contributor Author

ciriarte commented May 6, 2016

@heidermatos I think it needs a bit more testing, but it looks OK to me so far.

@valorkin
Copy link
Member

valorkin commented May 7, 2016

@ciriarte you can bother me with such PR's as much as you can :)

@@ -1,3 +1,5 @@
/// <reference path="node_modules/@angular/platform-browser/index.d.ts" />

This comment was marked as off-topic.

@valorkin
Copy link
Member

valorkin commented May 7, 2016

looks good, with some comments :)

@ciriarte
Copy link
Contributor Author

ciriarte commented May 7, 2016

@valorkin Thanks for the feedback! I've applied the changes you've requested. My only concern is the following npm warning that appears due to the shift to dev dep of rxjs:

npm WARN @angular/core@2.0.0-rc.1 requires a peer of rxjs@5.0.0-beta.6 but none was installed.

I guess we should leave it to the user to declare those dependencies in their project.json, according to this commit:

angular/angular@80b025a

Therefore, I've decided to just instruct Travis to install rxjs during build time.

'reflect-metadata',
'angular2/common',
'angular2/core'
'@angular/common',

This comment was marked as off-topic.

@ciriarte
Copy link
Contributor Author

ciriarte commented May 7, 2016

@valorkin done 👍 it did speed up the build a lot! I've removed 4 as well. Thanks for your feedback, I'm learning a lot.

@valorkin valorkin merged commit 13c5c35 into valor-software:master May 7, 2016
@rodolfocop
Copy link

very good man, I thank the commitment of all to finalize these modifications!

@valorkin
Copy link
Member

valorkin commented May 9, 2016

Hey, guys I am flying back, tomorrow I should be able to test and merge

On Sun, May 8, 2016, 17:04 Rodolfo Galli notifications@github.com wrote:

very good man, I thank the commitment of all to finalize these
modifications!


You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub
#176 (comment)

@rodolfocop
Copy link

No problem, i'll be waiting and if you need to be here to help!

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

4 participants