Skip to content

Conversation

@wwqrd
Copy link
Contributor

@wwqrd wwqrd commented Dec 10, 2021

Adds support for automatic layout on Sociogram: complexdatacollective/Interviewer#1156

New property added to behaviours in interface definition, which is structured as an object to allow more parameters in future:

behaviours: {
  automaticLayout: {
    enabled: true,
  },
  ...
}
...

Copy link
Member

@jthrilly jthrilly left a comment

Choose a reason for hiding this comment

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

Do we also need a migration for this, even if it is just blank and contains the info about the new functionality that it enables?

"items": {
"$ref": "#/definitions/Behaviours"
},
"minItems": 1
Copy link
Member

Choose a reason for hiding this comment

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

Other similar object keys don't have a minItems. I'm not really sure if they should be updated, or if this one shouldn't have any minimum. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that was odd too. This value hasn't been changed from schema 5. Perhaps we should open an issue to remove it where it isn't needed?

schemas/6.json Outdated
},
"Behaviours": {
"type": "object",
"additionalProperties": false,
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking perhaps we should set this to true so that future behaviour implementations don't require a new schema version. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, yes it does seem a bit restrictive.

I think so long as interfaces can work without properties being defined then that would be okay.

Maybe Interviewer could flag unknown additonal properties?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think that's a better way of doing it.

@jthrilly jthrilly merged commit 6b75a0c into master Jan 6, 2022
@jthrilly jthrilly deleted the feature/schema-version-6 branch October 9, 2023 12:08
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.

4 participants