Skip to content

Conversation

@warpech
Copy link
Contributor

@warpech warpech commented Mar 19, 2013

Thanks for a great library!

I made a few changes that make 100% tests from https://github.com/mikemccabe/json-patch-tests (master) passing.

Works for me so may work for others as well.

@mikemccabe
Copy link
Owner

Thank you! I'll try to review it soon. Have you checked it against https://github.com/json-patch/json-patch-tests ?

@warpech
Copy link
Contributor Author

warpech commented Mar 20, 2013

Sorry, I only tested against https://github.com/mikemccabe/json-patch-tests. I didn't realize that the other repo is now the leading one.

I made my changes because I needed as PHP counterpart to the JavaScript implementation I contributed to (Starcounter-Jack/JSON-Patch)

@bendiy
Copy link
Contributor

bendiy commented Jun 14, 2013

I merged this Pull Request into #3 and ran the tests against https://github.com/json-patch/json-patch-tests. Works fine.

@bendiy
Copy link
Contributor

bendiy commented Jun 14, 2013

JsonPatch::diff($doc1, $doc2);

@warpech This is still returning Draft 2 style patches after merging in your changes:

[
  {
    "add":"\/comments\/0",
    "value":{
      "text":"test",
      "created":"2013-06-14T18:50:21.904Z",
    }
  }
]

I'll update diff() to the final RFC style next week.

It should look like:

[
  {
    "op":"add",
    "path":"\/comments\/0",
    "value":{
      "text":"test",
      "created":"2013-06-14T18:50:21.904Z",
    }
  }
]

This does seem to work fine when applying final RFC style patches:

JsonPatch::patch($doc1, $patch);

@bendiy
Copy link
Contributor

bendiy commented Jun 17, 2013

diff() is now working as expected. See #3

bendiy referenced this pull request in bendiy/json-patch-php Apr 17, 2014
Switch to RFC branch
@mikemccabe mikemccabe merged commit df80c25 into mikemccabe:master May 31, 2014
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.

3 participants