Skip to content

Conversation

@franklinsch
Copy link
Contributor

@franklinsch franklinsch commented Oct 29, 2021

Bug/issue #, if applicable: rdar://83667105, SR-15354

Summary

Update the render JSON spec for the changes introduced in #11.

Dependencies

None.

Testing

There should be no user-facing changes. Verify that the specification matches the JSON produced by Swift-DocC.

Checklist

  • [ ] Added tests N/A
  • Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary

Update the render JSON spec for the changes introduced in swiftlang#11.

SR-15354
rdar://83667105
@franklinsch franklinsch force-pushed the r82919099-variantoverrides-spec branch from e5ac6e8 to 6c50150 Compare October 29, 2021 13:36
Copy link
Contributor

@mportiz08 mportiz08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

I have one suggestion for improving the JSON Patch details.

"JSONPatch": {
"type": "array",
"items": {
"type": "object",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible improvement might be to create explicit types for each distinct operation kind so that we can more clearly define which ones take which arguments (as opposed to just using the enum for the op field).

While each operation technically requires a path argument, some of them require a value and others require a from (etc). It is unlikely to occur in practice, but this spec could technically validate some invalid structures like

{
  "op": "test",
  "path": "/a/b/c",
  "from": "foo"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! I updated the spec with oneOf values.

Use oneOf to specify what properties are expected for each JSON patch
operation.

rdar://82919099
@franklinsch franklinsch requested a review from mportiz08 November 1, 2021 14:31
@franklinsch
Copy link
Contributor Author

@swift-ci please test

@franklinsch franklinsch merged commit 8da9448 into swiftlang:main Nov 1, 2021
@franklinsch franklinsch deleted the r82919099-variantoverrides-spec branch November 1, 2021 17:38
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.

2 participants