Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix add patch #206

Merged
merged 2 commits into from Jun 7, 2019
Merged

fix add patch #206

merged 2 commits into from Jun 7, 2019

Conversation

@Kimtho
Copy link
Contributor

Kimtho commented Jun 5, 2019

No description provided.

@gregsdennis

This comment has been minimized.

Copy link
Owner

gregsdennis commented Jun 5, 2019

Uhhh.... Did I get that backwards? Ha. Thanks.

@gregsdennis

This comment has been minimized.

Copy link
Owner

gregsdennis commented Jun 5, 2019

@Kimtho looks like it's failing one of the test suite tests.

@@ -84,8 +84,20 @@ private JsonPatchResult _Add(JsonValue json)

private JsonPatchResult _Remove(JsonValue json)
{
var (target, key, index, found) = JsonPointerFunctions.ResolvePointer(json, Path);
if (!found) return new JsonPatchResult(json, $"Path '{Path}' not found.");
if (Path == "")

This comment has been minimized.

Copy link
@gregsdennis

gregsdennis Jun 7, 2019

Owner

Please fix indentation. Tabs, not spaces. If you have Resharper or use Rider, the settings should load with the solution.

Manatee.Json/Patch/JsonPatchAction.cs Outdated Show resolved Hide resolved
if (Path == "")
{
//remove everything from the document
foreach (var item in json.Object.Keys.ToList())

This comment has been minimized.

Copy link
@gregsdennis

gregsdennis Jun 7, 2019

Owner

How about json.Object.Clear()?

@gregsdennis

This comment has been minimized.

Copy link
Owner

gregsdennis commented Jun 7, 2019

Thanks for helping to improve the library! Just a few items, please.

@Kimtho Kimtho force-pushed the Kimtho:Fix-patch-add branch 4 times, most recently from c7ecab5 to fa9ca5d Jun 7, 2019
@Kimtho Kimtho force-pushed the Kimtho:Fix-patch-add branch from fa9ca5d to e60a49f Jun 7, 2019
@gregsdennis gregsdennis changed the base branch from master to feature/175-draft-08 Jun 7, 2019
@gregsdennis gregsdennis changed the base branch from feature/175-draft-08 to master Jun 7, 2019
@gregsdennis gregsdennis changed the base branch from master to release/10.1.4 Jun 7, 2019
@gregsdennis

This comment has been minimized.

Copy link
Owner

gregsdennis commented Jun 7, 2019

Thanks for the updates. I'm going to merge this into a release branch so I can make the version updates. Expect a release today or tomorrow.

@gregsdennis gregsdennis merged commit 4f1e372 into gregsdennis:release/10.1.4 Jun 7, 2019
1 check passed
1 check passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.