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

Removing dependencies? #442

Closed
epoberezkin opened this issue Oct 14, 2017 · 8 comments
Closed

Removing dependencies? #442

epoberezkin opened this issue Oct 14, 2017 · 8 comments

Comments

@epoberezkin
Copy link
Member

@handrews could you please point me to the discussion about adding the note about the possibility of removing dependencies?

If it wasn't discussed, I would propose to remove this note from the spec until such possibility is agreed.

Without "dependencies", a concise schema:

{
  "dependencies": {
    "foo": ["bar"],
    "baz": ["quux"]
  }
}

will have to be replaced with:

{
  "allOf": [
    {
      "if": {"required": ["foo"]},
      "then": {"required": ["bar"]},
    },
    {
      "if": {"required": ["baz"]},
      "then": {"required": ["quux"]},
    }
  ]
}

which seems both more verbose and more difficult to understand.

So I am not sure what could be the motivation to remove it.

@handrews
Copy link
Contributor

You were the one who OK'd it:
#180 (comment)

The reason for putting the comment in is to provoke reactions from people who are actually using the keyword and want to keep it. The note is doing what it is intended to do. It is not in any way a commitment to removing the keyword.

I'm closing this not because your objections aren't useful (they are, and I've noted the point), but because I want people other than the usual suspects to file objections. That works better if they can't find one that's already open :-)

@handrews
Copy link
Contributor

Thinking on this a bit more, it's really the schema form of dependencies that I would prefer to remove. I agree that the property name form is a nice shortcut. But I don't think that "if": {"required": ["foo"]} on its own, with an arbitrary then, is worth having a shortcut for.

Or if it is, I really don't like having the same keyword used for the string and schema forms. They're not really the same thing and it's hard to reason about the keyword as it can be either a subschema keyword or a value keyword which is weird.

@handrews handrews reopened this Oct 15, 2017
@handrews
Copy link
Contributor

type has two forms, but both are value forms (single string vs array of strings). items has two forms, but both are subschema forms (single subschema vs array of subschemas).

dependencies has one value form and one subschema form. I think the value form is worth keeping, but I'd like to encourage removal of the subschema form in favor of if. Perhaps I could clarify the note in this manner?

@epoberezkin
Copy link
Member Author

epoberezkin commented Oct 15, 2017

I agree about schema form, this one was a bit confusing indeed.

I think I meant schema form in that comment too ;)

@pipobscure
Copy link

I find the schema form very useful. Please let's not remove it. If anything let's remove the values form.

{ "foo": { "required": [ "bar" ] } }

isn't that much worse than

{ "foo": [ "bar" ] }

but replacing the schema version with a combination of allOf/anyOf/oneOf/if/then/else is much worse.

It's much clearer than if/then/else.

Side-jibe at if/then/else: These are a bad idea to begin with, but since they are not obligatory to use, I'm fine keeping them. One reason why they are bad is that they mutate JSON-Schema from a schema to a programming language.

@handrews
Copy link
Contributor

One reason why they are bad is that they mutate JSON-Schema from a schema to a programming language.

This was covered in excruciating detail in #180, and will not be revisited until the keywords have been available in a published draft to see how they are or are not used "in the wild."

@handrews handrews added this to the draft-future milestone Oct 18, 2017
@handrews handrews changed the title Removing dependenices? Removing dependencies? Oct 31, 2017
@markchart
Copy link

Although I am a big fan of "if","then","else" I would not care to lose (either form of) "dependencies". Some constraints may be expressed much more clearly using "dependencies" (just as some things are better expressed using "if","then","else" than tangled "xxxOf" clauses).

It is counterproductive to try to squeeze out all redundancies at the cost of clarity, not just of exposition but of intent.

@handrews
Copy link
Contributor

Based on feedback we opted to split dependencies into two keywords rather than remove it. This addressed the problematic aspects of the keywords without losing functionality.

The string array form is now requiredDependencies, while the schema form will either remain dependencies or become schemaDependencies (I lean towards the latter, as it makes compatibility easier by not changing an existing keyword- see #591 for discussion).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants