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

@pavelvasev merge: Aliases now emit changes signals #195

Merged
merged 2 commits into from Apr 23, 2016

Conversation

ChALkeR
Copy link
Member

@ChALkeR ChALkeR commented Apr 18, 2016

This merges 780450b and the aliases part of dce7d96.

Note: I have not reviewed this code.
Note: This fixes two testcases. It also blocks sorting out the two remaining changestets of #32, so it would be better to review this quickly =).

Ref: #32.

/cc @pavelvasev @Plaristote @akreuzkamp

@ChALkeR ChALkeR mentioned this pull request Apr 18, 2016
62 tasks
@ChALkeR ChALkeR added the merge label Apr 18, 2016
@ChALkeR ChALkeR force-pushed the merge-paverlvasev-aliassignals branch from 1e3057b to 9b4f0f3 Compare April 22, 2016 06:24
@ChALkeR
Copy link
Member Author

ChALkeR commented Apr 22, 2016

Updated, a second aliases-related commit added, a second testcase is now fixed by this changeset 😉.

@ChALkeR ChALkeR force-pushed the merge-paverlvasev-aliassignals branch from 9b4f0f3 to c31ec78 Compare April 22, 2016 06:29
targetProp.changed.apply( obj, arguments );
loopWatchdog = false;
} );
} ) ();
Copy link
Member Author

@ChALkeR ChALkeR Apr 22, 2016

Choose a reason for hiding this comment

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

Note: code style is not perfect here, e.g. ( function() { wrap is not needed. But we can fix that later at the cleanup stage, I would want to keep it like this until we complete the merges.

@ChALkeR
Copy link
Member Author

ChALkeR commented Apr 22, 2016

This does LGTM, the change here is additive.
Going to land this today if there would be no objections.

PR-URL: #195
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
PR-URL: #195
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@ChALkeR ChALkeR force-pushed the merge-paverlvasev-aliassignals branch from c31ec78 to b0df2f3 Compare April 23, 2016 15:22
@ChALkeR ChALkeR merged commit b0df2f3 into master Apr 23, 2016
@ChALkeR ChALkeR deleted the merge-paverlvasev-aliassignals branch April 23, 2016 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants