Switch from add to replace/update properties #32
Conversation
|
Typo in lorna's message there, she meant to say:
Basically nicking the relevant and useful concepts from JSON Patch RFC 6902 and their list of operations: https://www.rfc-editor.org/rfc/rfc6902.html#section-4 |
versions/1.0.0.md
Outdated
| target | `string` | **REQUIRED** A JSONPath query expression referencing the target objects in the target document. | ||
| description | `string` | A description of the action. [CommonMark syntax](https://spec.commonmark.org/) MAY be used for rich text representation. | ||
| add | Any | An object with the properties and values to be merged with the object(s) referenced by the `target`. If an object property exists, it is overwritten with the new value. Array elements are appended to any existing entries already in the array. This property MUST NOT be used with the `replace` property in the same action object. This property has no impact if `remove` property is `true`. | ||
| replace | Any | An object with the properties and values to be merged with the object(s) referenced by the `target`. The entire contents of the target object or array is replaced by the new value(s) supplied. This property MUST NOT be used with the `add` property in the same action object. This property has no impact if `remove` property is `true`. |
There was a problem hiding this comment.
Is this right? It says the values will be merged into the target then it the values will be replaced in the target.
There was a problem hiding this comment.
Agreed, the first sentence for replace seems to contradict the second.
There was a problem hiding this comment.
Updated, could you check this makes more sense?
| @@ -111,10 +111,11 @@ This object represents one or more changes to be applied to the target document | |||
|
|
|||
| Field Name | Type | Description | |||
| ---|:---:|--- | |||
| <a name="actionTarget"></a>target | `string` | **REQUIRED** A JSONPath query expression referencing the target objects in the target document. | |||
| <a name="actionDescription"></a>description | `string` | A description of the action. [CommonMark syntax](https://spec.commonmark.org/) MAY be used for rich text representation. | |||
| <a name="actionUpdate"></a>update | Any | An object with the properties and values to be merged with the object(s) referenced by the `target`. This property has no impact if `remove` property is `true`. | |||
There was a problem hiding this comment.
Is it worth keeping update aroud for a minute as deprecated or is this early and experimental enough that whatever?
There was a problem hiding this comment.
This is always a hard decision to make. At this point in time, I'd vote to just kill it. We won't always have this luxury, but we should make use of it while we can.
There was a problem hiding this comment.
We'll always have the commit history, but this does make me wonder if we should propose a 0.1.0 version and then start following a more formal release process. We're already seeing usage ....
There was a problem hiding this comment.
.... Alternatively, how about moving forward from our current situation by making this a 1.1.0 and still a draft?
|
Really appreciate having some parallels with JSON Patch! |
Thanks! Co-authored-by: Darrel <darrmi@microsoft.com>
I'm proposing this change as a response to the ongoing discussion in #30 . Using "add" can be confusing and leaves us without being able to support some use cases, such as setting an array to a particular set of elements. Looking around for similar-ish approaches, we found that JSONPath uses "update" for our existing "add" operation, and "replace" to achieve another sort of adding something. I've therefore proposed a specification update to adopt the new terminology.
It's a big change and while sooner is of course the best time to be making these changes, I'd appreciate as many eyes and as much feedback as possible. Comments and questions welcome.