search

aspnet / JsonPatch

[Archived] JSON PATCH library. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
103 stars 48 forks source link

Improve conformance of replace operations to spec #111

Closed TAGC closed 7 years ago

TAGC commented 7 years ago

Addresses #110

This change makes it so that "replace" operations will remove existing values at a path before setting the new value.

Submitting this PR partly because I have a colleague desperately waiting on a fix that's blocked by this issue, but mainly because this should count as 1 of my 4 for Hacktoberfest.

I've modified an existing unit test to expose the bug and confirm the fix. I've also confirmed these changes fix the issue I was having.

dnfclas commented 7 years ago

@TAGC, Thanks for having already signed the Contribution License Agreement. Your agreement has not been validated yet. We will now review your pull request. Thanks, .NET Foundation Pull Request Bot

rynowak commented 7 years ago

@jbagga - can you please take a look at this?

jbagga commented 7 years ago

Closing this for now as the current implementation supports the generic scenario.

TAGC commented 7 years ago

Kind of disappointing but fair enough. Would it be okay (legally) to push a NuGet package with these changes to my company’s private MyGet feed?

Eilon commented 7 years ago

The spec doesn't dictate how an implementation needs to implement the replace operation, rather just that the ultimate object should be in a particular state after the operation has completed, so I don't think there's an issue there.

But, so long as you abide by the license used by this project (Apache 2.0 license), you are free to make any changes you want to a copy of the code and publish to your own feed.