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

WIP fix(ngMessages): fix orphaned ng-message comments when using ng-if #16404

Conversation

CodySchaaf
Copy link

Pull request for help on issue #16389

Fix an issue where if you remove ng-message comments with ng-if
before they are attached to the dom you will get an error when ngMessages
tries to reveal them.
@googlebot
Copy link

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!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

1 similar comment
@googlebot
Copy link

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!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@frederikprijck
Copy link
Contributor

frederikprijck commented Jan 11, 2018

@CodySchaaf It looks like your commit author isn't linked to your github account, you probably wanna fix that, despite this PR getting merged or not (it's useful for other PR's/repos aswel)

@Narretz
Copy link
Contributor

Narretz commented Jan 11, 2018

@CodySchaaf the PR should have the master branch as merge target, not 1.5.x. We merge all PRs into master first, because that's the most up to date code. We actually don't update 1.5.x. anymore, only master and 1.6.x

Can you also add a test case in ngMessagesSpec that is fixed by this change? You should be able to adapt one of the tests.

@Narretz Narretz added this to the Ice Box milestone Jan 11, 2018
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jan 12, 2018
@petebacondarwin
Copy link
Contributor

I think #16406 should fix this issue

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jan 17, 2018
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jan 17, 2018
@Narretz Narretz closed this in 50ceb23 Jan 17, 2018
Narretz pushed a commit to Narretz/angular.js that referenced this pull request Jan 17, 2018
Narretz pushed a commit that referenced this pull request Jan 17, 2018
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.

5 participants