This Fixes #85 'angular.merge is not a function .controller'

[dtaylor113]

No description provided.

[waldenraines]

Need some tests for pfUtils.

[waldenraines]

Also, why don't we just use angular.extend() instead of angular.merge()?

[jeff-phillips-18]

The extendDeep copy does not clone the second argument's (defaultConfig) fields when copying to the dst object. This causes the defaultConfig to be modified if the dest object is modified for fields that are objects or arrays.

[dtaylor113]

"Also, why don't we just use angular.extend() instead of angular.merge()?"

angular.extend() doesn't do a deep copy.

[dtaylor113]

"Need some tests for pfUtils."
There chart and view objects which use pfUtils do have unit tests which verify pfUtils is working correctly.

[waldenraines]

There chart and view objects which use pfUtils do have unit tests which verify pfUtils is working correctly.

No they don't, they verify that angular.merge() is working correctly (or in the case of 1.3, _.merge). If we're going to keep this approach it needs to be tested, especially this new extendDeep() function.

In dev mode, I commented out the other choices so that 'extendDeep()' was being used. All unit tests and ng-doc examples ran successfully, but I agree this is not enough of a test. I am working on unit tests for this. -thanks

[waldenraines]

angular.extend() doesn't do a deep copy.

Then what about _.extend()? Since we're requiring underscore can't we just use that and get rid of the need for pfUtils?

[dtaylor113]

Underscore extend is a shallow copy, we need a deep copy. pfUtils is need for those customers who are < angular-js 1.4 and are using underscore (not lodash). In this case, we need to make our own 'extendDeep' function.

[erundle]

Would prefer to see this called merge but otherwise LGTM

[waldenraines]

Underscore extend is a shallow copy, we need a deep copy. pfUtils is need for those customers who are < angular-js 1.4 and are using underscore (not lodash). In this case, we need to make our own 'extendDeep' function.

How long will we support < angular 1.4? Should we file an issue to get rid of pfUtils at some point in the future? I don't think we want to support this indefinitely and it introduces extra dependencies that will not be needed once people upgrade.

[dtaylor113]

Hi, I believe my latest push addressed all issues.
If so, @jeff-phillips-18 please merge
-thanks

[dtaylor113]

Hi @jeff-phillips-18, latest push address you last comment -thanks