X Tutup
Skip to content

build(npm): update rxjs to 2.0.0-beta.6#8047

Closed
jeffbcross wants to merge 1 commit intoangular:masterfrom
jeffbcross:pr-8003
Closed

build(npm): update rxjs to 2.0.0-beta.6#8047
jeffbcross wants to merge 1 commit intoangular:masterfrom
jeffbcross:pr-8003

Conversation

@jeffbcross
Copy link
Copy Markdown
Contributor

Note: this contains two commits from #8003 to make sure Travis will be green.

@jeffbcross
Copy link
Copy Markdown
Contributor Author

I think I need to fix a couple of typings in http

@jeffbcross
Copy link
Copy Markdown
Contributor Author

There are obscure Travis failures in environments I don't have time to troubleshoot right now, so please don't merge this unless someone can make it green.

@jeffbcross jeffbcross added state: blocked and removed action: merge The PR is ready for merge by the caretaker pr_state: LGTM labels Apr 20, 2016
@jeffbcross
Copy link
Copy Markdown
Contributor Author

I'm going to try reproducing and fixing the Windows and Safari issues now

@jeffbcross
Copy link
Copy Markdown
Contributor Author

@Blesh had the idea that our Symbol problems are probably the result of RxJS no longer polyfilling Symbol as it did in beta.2, and our code in facade/lang.ts was assuming that Symbol had been polyfilled when calling isPresent(Symbol) in lang.ts:

if (isPresent(Symbol) && isPresent(Symbol.iterator)) {

I changed that line to if ('Symbol' in globalScope && isPresent(Symbol.iterator)) {

@jeffbcross jeffbcross added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: blocked labels Apr 21, 2016
@jeffbcross jeffbcross added the action: merge The PR is ready for merge by the caretaker label Apr 21, 2016
@jeffbcross jeffbcross assigned robertmesserle and unassigned alxhub Apr 21, 2016
@jeffbcross jeffbcross added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 21, 2016
@jeffbcross
Copy link
Copy Markdown
Contributor Author

Got verbal LGTM from @alxhub

@robertmesserle this will require updating internal version of Rx, which I can help with. It'd be great to get this landed before our next beta release.

@benlesh
Copy link
Copy Markdown
Contributor

benlesh commented Apr 21, 2016

@jeffbcross going forward, if you need to access Symbol.observable you'll want to use this module: https://www.npmjs.com/package/symbol-observable

@rkirov
Copy link
Copy Markdown
Contributor

rkirov commented Apr 25, 2016

This is blocked on google internal ts1.9 upgrade. Will revisit after ng-conf.

@benlesh
Copy link
Copy Markdown
Contributor

benlesh commented Apr 25, 2016

Just noticed this:

update rxjs to 2.0.0-beta.6

That would be a 3 major version downgrade.

@jeffbcross
Copy link
Copy Markdown
Contributor Author

I'm going to rebase and get this thing ready for merge. We're not going to wait on G3. This is blocking some offline compiler work now.

@jeffbcross
Copy link
Copy Markdown
Contributor Author

Thx @Blesh fixed commit message :)

@jeffbcross jeffbcross assigned rkirov and unassigned robertmesserle Apr 26, 2016
@jeffbcross
Copy link
Copy Markdown
Contributor Author

READY TO MERGE!

@mary-poppins
Copy link
Copy Markdown

Merging PR #8047 on behalf of @rkirov to branch presubmit-rkirov-pr-8047.

@PatrickJS
Copy link
Copy Markdown
Contributor


:

@mhevery mhevery closed this in b62bccf Apr 26, 2016
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

X Tutup