Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): bindToController should work without controllerAs #15110

Closed

Conversation

thorn0
Copy link
Contributor

@thorn0 thorn0 commented Sep 8, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

fix

What is the current behavior? (You can also link to an open issue here)

see #15088

Does this PR introduce a breaking change?

no

Please check if the PR fulfills these requirements

@Narretz
Copy link
Contributor

Narretz commented Sep 8, 2016

Looks like we can decide between this and #15106

@thorn0
Copy link
Contributor Author

thorn0 commented Sep 8, 2016

Yep, I didn't see #15106

@gkalpak
Copy link
Member

gkalpak commented Sep 8, 2016

Let's keep this one. It's more complete than #15106.

@thorn0
Copy link
Contributor Author

thorn0 commented Sep 8, 2016

Is there a way to restart Travis without pushing a new commit?

@gkalpak
Copy link
Member

gkalpak commented Sep 8, 2016

There is 😄

expect($rootScope.myCtrl).toBeUndefined();
} else {
// The controller identifier was added to the containing scope!
// Should we allow this to happen?
Copy link
Member

Choose a reason for hiding this comment

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

Why not? It's their code, they are allowed to do things 😃
In any case, this is not related to this PR. Remove this comment and feel free to open a new issue about it if you feel it shouldn't happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wasn't sure about it...

@gkalpak
Copy link
Member

gkalpak commented Sep 8, 2016

LGTM as long as Travis is happy (and unless someone can think of a good reason not to allow this).

@thorn0 thorn0 force-pushed the bindToController_minus_controllerAs branch from c2532c6 to 9037111 Compare September 8, 2016 16:10
@Narretz
Copy link
Contributor

Narretz commented Sep 15, 2016

LGTM, please merge

@gkalpak gkalpak closed this in 16dccea Sep 16, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants