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

Allow scripts and targets in schema to be permissive #644

Closed
wants to merge 1 commit into from

Conversation

devnote-dev
Copy link
Contributor

Geode makes use of custom properties in these fields which show as invalid in the editor. In future it would probably be a good idea to also make other parts of the schema permissive.

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Oct 3, 2024

Well, they're custom extensions that don't conform to the SPEC. The schema error is right to outline these as such.

Maybe the SPEC could evolve to allow for some extensions (in a future compatible way).

@devnote-dev
Copy link
Contributor Author

Geode's primary use case for targets is allowing a flags: property to be set which is passed with the command when building a target, and for scripts is allowing custom scripts (platform-dependent or not) to be ran, including an extension of postinstall for platform support.

These are of course subject to change during development so I'm not sure how the SPEC could be changed to allow extensions, besides just labeling them unsupported by Shards.

@straight-shoota
Copy link
Member

straight-shoota commented Oct 7, 2024

I don't think we can allow arbitrary additional properties in the schema. They could conflict with future properties added to the specification, undermining its purpose.
Shards itself does not prevent you from adding properties and it'll happily ignore anything it doesn't recognize. This is probably reasonable behaviour in the interest of maximum compatibility. However we should not bless any additions outside the specification as being conform with the the schema.

@devnote-dev devnote-dev deleted the shard-yml-json branch October 13, 2024 11:57
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