Skip to content

Add "$ref" for subschemas. #188

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

Closed
wants to merge 1 commit into from
Closed

Conversation

handrews
Copy link
Contributor

This is the same approach used in the PR for Draft 05 meta-schemas
in the web site repo.

Copy link
Member

@Relequestual Relequestual left a comment

Choose a reason for hiding this comment

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

This makes sense

@Relequestual
Copy link
Member

Anyone seeking an explanation to this should see #168 (comment)

@Relequestual
Copy link
Member

@awwright review requested! =]

@awwright
Copy link
Member

It looks like as of PR #167 there's no longer separate behavior for subschemas.

Unless there's something I'm missing... can you point out the language that proscribes this behavior?

@handrews
Copy link
Contributor Author

PR #186 was the first commit from #168
PR #187 was the second commit from #168
This PR #188 was the third commit from #168

Which is why they were stacked in #168 in the first place- they make a logical sequence and they could not be merged separately without conflicts. That wasn't for my own amusement. There is nothing here that you haven't seen before.

@Relequestual Relequestual added this to the Meta-schema draft-05 milestone Dec 13, 2016
@Relequestual
Copy link
Member

@awwright My understanding is that you're missing #168 (comment)

@awwright
Copy link
Member

@handrews I'm not sure what you're responding to... I'm just trying to figure out which language it is that makes referring to <#/definitions/subSchema> necessary.

@Relequestual As far as I can tell, that only describes why $ref is mutually exclusive.

@handrews
Copy link
Contributor Author

@awwright See #174. I had previously not had "#/definitions/subSchema", but you made me change it because "$ref" is not allowed as a root schema. Therefore there is a difference between a root schema and a subschema. Therefore we need to define subschema somewhere because most previously references to "#" need the subschema definition.

@awwright
Copy link
Member

@handrews iirc that was just a patch I had up for a few hours, I can't actually find that language in the current documents. Can you point it out please?

@handrews
Copy link
Contributor Author

@awwright you asked for this change, are you telling me you don't even know why you did? There was no "patch" for #174, you just told me to change this.

Anyway, the wording is in the obvious place, which is section 7 where "$ref" is defined:

Any time a subschema is expected, a schema may instead use an object containing a "$ref" property.

@awwright
Copy link
Member

awwright commented Dec 13, 2016

@handrews You're seeing that in jsonschema-core, section 7? That's strange... the current "master" branch for me shows this:

7.  Schema references with $ref

	The "$ref" keyword is used to reference a schema, and provides the
	ability to validate recursive structures through self-reference.

	An object schema with a "$ref" property MUST be interpreted as a
	"$ref" reference.  The value of the "$ref" property MUST be a URI
	Reference.  Resolved against the current URI base, it identifies the
	URI of a schema to use.  All other properties in a "$ref" object MUST
	be ignored.

Can you double-check? Or else I've just got to be going crazy, or maybe GitHub is being buggy again.

@handrews
Copy link
Contributor Author

@awwright I was looking on the last draft by accident, but really, I did this because you wanted it.

Apparently you don't want it anymore. Is that correct? I don't even care what language says what, I'm just trying to appease you at this point and get the changes in. I've re-done this commit four or five times now and you have a different change you want every time.

What will make you happy here? If I change it so that at the top level a schema can be a boolean, a regular object, or a $ref, will you accept that?

@awwright
Copy link
Member

@handrews Ok, glad that's been figured out. I asked for the separate PR in the hopes it would be helpful for stuff like this.

If it's apparent to you that this isn't necessary anymore, then you can just close out the issue.

Again, sorry about the confusion! I'd swear GitHub really has been acting funny lately.

@handrews
Copy link
Contributor Author

I asked for the separate PR in the hopes it would be helpful for stuff like this.

This had nothing to do with a separate PR. I walked you through this change in #186. At that point, it did not involve "subSchema". You objected to allowing "$ref" in a root schema (on IRC, so of course no one else saw it nor can I refer to it) and made me change the commit (which I did) and file #174. And then you refused to look at the changes that I made at your request and closed the PR on me. I have done so much work at your request on this whole series of changes, and you just keep moving the goal every single time.

If it's apparent to you that this isn't necessary anymore, then you can just close out the issue.

Where do you even get this? Also, this is a yes-or-no question that I asked which you completely ignored:

If I change it so that at the top level a schema can be a boolean, a regular object, or a $ref, will you accept that?

Something is still needed here. The only thing that's clear to me is that no matter where I put this commit or how I arrange it, you are not happy with it. But you won't answer what will make you happy.

@handrews
Copy link
Contributor Author

@awwright l've filed #194 as a non-subschema-specific alternative to this PR. Perhaps it is more to your liking?

@Relequestual Relequestual requested review from awwright and removed request for awwright December 14, 2016 10:28
@awwright
Copy link
Member

@handrews I don't recall my opinions on "$ref" being so strong. I didn't intend it to be an editorial objection. The things I preface with "In my opinion" or "I think" are just my personal opinions, they are subject to change in the face of a compelling argument to the contrary -- and frequently do. I'm terribly sorry if that's made things confusing recently.

Where do you even get this?

I'm merely saying if. If you don't think this patch is necessary anymore, please close it out. If you think it is, please let me know which language warrants its inclusion.

If I change it so that at the top level a schema can be a boolean, a regular object, or a $ref, will you accept that?

This, like most things, is subject to opening an issue/PR and convincing people that it's necessary to do. I don't have a personal opinion myself right now.

If you think you have a solution to a problem, by all means, open an issue/PR and let's see it! Thanks for #194!

This is the same approach used in the PR for Draft 05 meta-schemas
in the web site repo.
@Relequestual
Copy link
Member

I'm closing this in favour of #194, which I think is easier to understand at first glance than this solution.

@handrews handrews mentioned this pull request Dec 27, 2016
@handrews handrews deleted the meta3 branch January 5, 2017 19:42
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