Add delete operation
#25
Conversation
|
@manuelstofer @epoberezkin Can you please review this PR? |
|
@IvanGoncharov ok with some little corrections. Please also merge changes from master into that branch. @manuelstofer, can you please set branch protection on master branch like this:
You won't have coveralls though, but it's ok, we will add it later. Or you can give me admin rights if you like... |
Readme.md
Outdated
|
|
||
| Removes an attribute of object referenced by pointer. | ||
| Removes an attribute of object or an item from array referenced by pointer. | ||
| **Note**: Since `v0.6.0` this function changed handles arrays differently, see # . |
There was a problem hiding this comment.
s/changed handles/handles, also issue number
There was a problem hiding this comment.
@epoberezkin Done. 👍
|
|
||
| ### .delete(object, pointer) | ||
|
|
||
| Use JS `delete` operator on an attribute of object or an item from array referenced by pointer. |
There was a problem hiding this comment.
Please add note that delete works in the same way as remove before 0.6.0
There was a problem hiding this comment.
@epoberezkin I don't think it will be useful to anybody. If you new to this library it just confuses you. If used it prior 0.6.0 and want to know why started to behave differently remove you will check description of remove.
deleteisn't a hack to simplify migration it's has it own semantic. And useful on it's own without any ties to0.6.0.
|
@epoberezkin Thanks for all your effort!! I enabled branch protection |
|
@epoberezkin I rebased and address your PR comments |
|
@epoberezkin I addressed your comment. |
|
@IvanGoncharov I expected you to add tests for failing cases. Coverage should stay the same. To merge as is I'll have to remove coveralls criteria and re-run the job - I don't have permission to merge regardless (and I don't want to, to be honest :)... |
@manuelstofer @epoberezkin I implemented what we agreed on in #23.
Can you please review it?