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

docs(tutorial/10 - Event Handlers): missing unit test #13152

Closed
wants to merge 1 commit into from

Conversation

Airborn22
Copy link

In the doc there stands "You also have to refactor one of your unit tests because of the addition of the mainImageUrl
model property" but the spec was missing the test of mainImageUrl

In the doc there stands "You also have to refactor one of your unit tests because of the addition of the `mainImageUrl`
model property" but the spec was missing the test of `mainImageUrl`
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case 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. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@Airborn22
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

Airborn22 added a commit to Airborn22/angular-phonecat that referenced this pull request Oct 22, 2015
There was a missing test for `mainImageUrl`. There is a pull request as well for the doc: angular/angular.js#13152
@Airborn22
Copy link
Author

There is a pull request for the phonecat app but it's failing now because of an issue on master: angular/angular-phonecat#279

@Narretz
Copy link
Contributor

Narretz commented Nov 2, 2015

Thanks for the PR. The way I see it, the description is accurate: You need to update the test, because the phone on the scope is different now. We can add an additional expect/test for the mainImageUrl, but it is already tested in the e2e test.

@Narretz Narretz added this to the Ice Box milestone Nov 2, 2015
@Airborn22
Copy link
Author

Thank you @Narretz, I created a pull request for the test but all the builds are failing there for some reason. I will create a new pull request later.

@Narretz
Copy link
Contributor

Narretz commented Nov 3, 2015

You don't have to open a new PR, you can push --force to your branch and it iwll update the PR.

@gkalpak
Copy link
Member

gkalpak commented May 26, 2016

The tutorial has changed quite a bit (with #14416).
If you think this change is still relevant, please submit a new PR against the latest code.

Thx !

@gkalpak gkalpak closed this May 26, 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

5 participants