fix typo in descriptions and add test to match blank object attribute #8004
Conversation
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
rolfyone
changed the title
|
I used that template - hopefully that's ok? sorry i'm a newbie :) |
|
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
CLA signed again |
|
the build failure looks nothing to do with my change, but i have no idea how to proceed... |
…ject attributes Two descriptions contain typo's to the word predicate. Also, to prove behavior in ticket 7890, create a new test to display filter Closes #7891
| @@ -61,7 +61,7 @@ describe('Filter: filter', function() { | |||
| }); | |||
|
|
|||
|
|
|||
| it('should support predicat object with dots in the name', function() { | |||
| it('should support predicate object with dots in the name', function() { | |||
There was a problem hiding this comment.
these aren't really related to the test addition
|
Hmm, well maybe we don't care in this case, but ordinarily I'd say split this into two commits, one is a style fix and the other is a test otherwise, lgtm I guess |
|
they were the 'fix typos' bit - i can remove them and put them as a separate ticket if preferred... |
|
so normally 2 commits on the same ticket? or separate ticket... this is my first github foray, so just not clear on the preference... |
|
Also, you should move the test under the There's an existing test, but it doesn't cover this particular case. It might not be wise to suggest people use this as angular.equals can be expensive if it's looking at objects instead of primitives. |
|
Two commits in one ticket is fine (in my opinion) if they're doing different things --- in my opinion, it's best when each commit does exactly one "thing", and all the changes in that commit are pertinent to that specific thing |
|
Ok that's a fair call re 1 commit per 'thing'. I'll move that test to the other section. |
…ect attribute to descriptor section Move the test to show matching empty string to the should support comparator section. Closes #7891
|
@caitp should be all good to go. not sure if i had to flag it or if the change is flagged internally so apologies if this is just an 'extra' message :) |
Two descriptions contain typo's to the word predicate. Closes #8004
Request Type: test
How to reproduce:
Component(s): misc core
Impact: small
Complexity: small
This issue is related to:
Detailed Description:
while investigating #7890 I found a couple of typos in the filter tests, and used the test suite to prove the problem was working as specified. There were no test cases specifically to show the behaviour that I could find, so committed it as an example of matching empty string through use of exact match.
Other Comments:
test(filter): fix typo in descriptions and add test to match blank object attribute
Two descriptions contain typo's to the word predicate.
Also, to prove behaviour in ticket 7890, create a new test to display filter
Closes #7891