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

Tap generate docs: anyOf support and improved error messages #476

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

luandy64
Copy link
Contributor

@luandy64 luandy64 commented Apr 21, 2020

Summary

This PR adds the following improvements to tap-generate-docs:

  • Add a helpful error message when we encounter an array with no items field
    • This mirrors the check we do for objects and properties
  • Add support for anyOf in schemas
  • Add a comment block that can be used to debug the script

I highly recommend hiding whitespace changes while reviewing the PR.

anyOf design

convert-multiary-type already supports schemas with a array for the type of the field.

["a_date" {"type" ["null" "string" "integer"]
"format" "date-time"}]
{"name" "a_date"
"type" "date-time, integer"
"description" ""}

So to support anyOf, this PR merges all of the maps in the anyOf array and lets the the existing functions handle the parsing.

Notice the test case I added to convert-multiary-type-tests mirrors the test case right above it (except mine uses anyOf) and the outputs are expected to be the same.

Outstanding questions

  • Is there a more clojure-y way to write maybe-remove-anyOf?
  • Is there a function that does the merge I want to do in remove-anyOf without my anonymous function passed to merge-with?

@luandy64 luandy64 marked this pull request as draft May 1, 2020 14:12
@luandy64 luandy64 force-pushed the tap-generate-docs/anyOf-support branch from 8fa22a4 to cfa00e0 Compare May 4, 2020 13:08
@luandy64 luandy64 force-pushed the tap-generate-docs/anyOf-support branch from cfa00e0 to 5d63021 Compare May 4, 2020 13:13
@luandy64 luandy64 marked this pull request as ready for review May 4, 2020 13:58
Copy link
Contributor

@dmosorast dmosorast left a comment

Choose a reason for hiding this comment

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

The general gist sounds good, and I trust that the tests prove that.

I'm concerned about the case where the type is something like "anyOf": [{"type": "string"}, {"type": "string", "format": "date-time"}], since merging them seems to remove the notion that this is a special case where the dates can return as invalid, in which case we type them as strings.

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