Skip to content
This repository was archived by the owner on Nov 3, 2023. It is now read-only.

Conversation

@Potherca
Copy link

@Potherca Potherca commented Oct 22, 2016

Let me know if I missed anything and feel free to squash the commits.

Fixes #48.

@embray
Copy link
Collaborator

embray commented Nov 8, 2016

Awesome, thanks for this (and apologies for the late reply--I've been on vacation).

I need to look into the Travis failure--I'm pretty sure it's unrelated to your PR.

I don't know much about using JSONSchema in PHP so I have to pretty much take your word for it, though based on what I do know about PHP everything looks like it makes sense.

Copy link
Collaborator

@embray embray left a comment

Choose a reason for hiding this comment

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

Some minor nitpicks aside looks good to me.

--Ruby
In Ruby, "boolean" is analogous to ``TrueClass`` and ``FalseClass``. Note
that in Ruby there is no ``Boolean`` class.
--Python
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you meant --PHP here

|boolean |Boolean |
+----------+--------------+
|null |NULL |
+----------+--------------+
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not really sure whether PHP types should be spelled in upper-case or lower-case. I don't think it really matters since the built-in types are not first-class objects, though gettype() returns the lower-case spelling. Either way I think one spelling should be used consistently (and in some of the examples below you use all lower-case instead).

@Potherca
Copy link
Author

Potherca commented Feb 4, 2018

Closing due to lack of activity.

@Potherca Potherca closed this Feb 4, 2018
@mdboom mdboom mentioned this pull request Aug 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants