Skip to content

Conversation

@mamcgrath
Copy link

Fixes handling of nested paths at the cost of adding a dependency on Lodash.

@coveralls
Copy link

coveralls commented Sep 30, 2019

Coverage Status

Coverage increased (+0.08%) to 94.483% when pulling 75e269d on mamcgrath:fix_handle_multiple_nested_paths into 832ca27 on grofers:master.

@jancama2
Copy link

@aditbiswas1 how does it look like with the MR? I kinda need this to be merged.

@jancama2
Copy link

@aditbiswas1 we need one more to merge it, could you pls find someone?

// expectations
expect(value.userId).toEqual(userId);
expect(value.auth.authToken).toEqual(authToken);
expect(value.auth.key).toEqual(key);
Copy link
Contributor

Choose a reason for hiding this comment

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

how is this test of auth.key any different from auth.authToken ?

Choose a reason for hiding this comment

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

With the current version of the library this would fail since there is no deep merge.

// expectations
expect(value.userId).toEqual(userId);
expect(value.auth.authToken).toEqual(authToken);
expect(value.auth.key).toEqual(key);

Choose a reason for hiding this comment

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

With the current version of the library this would fail since there is no deep merge.

@ashbhir
Copy link
Contributor

ashbhir commented Nov 27, 2019

Approved

Approved with PullApprove

@ashbhir ashbhir merged commit 9e9b70b into grofers:master Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants