-
Notifications
You must be signed in to change notification settings - Fork 64
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
[native_assets_cli] Hook schemas #2064
Conversation
PR HealthBreaking changes ✔️
Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
License Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
|
8975dd6
to
053a7fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to exactly review the schemas but LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of feedback with a few questions.
Bug:
This PR documents the current state of JSON protocol for the hooks by means of JSON schemas.
(It is a non-goal of this PR to change the current state.)
Features of the JSON schemas
The syntax:
CodeConfig
must have an"android"
config if the"target_os" : "android"
.Documentation for the evolution of schemas:
"deprecation"
sOrganization of the JSON schemas
package:hook
) and extensions (package:code_assets
andpackage:data_assets
).allOf
to get a schema with two extensions applied.shared_definitions.schema.json
files to facilitate easy re-use of subschemas occurring in multiple places (Asset
,Architecture
, etc.).build_input.schema.json
,build_output.schema.json
,link_input.schema.json
, andlink_output.schema.json
.hook
s or only relevant forsdk
s are added on to theshared
schema.Testing
The schemas are tested with the help of
package:json_schema
. This package only supports up to meta schema 07, so this is the version we use in our schemas.This package does not give useful error messages on validation errors, so we cannot use this for generating syntactic error messages unfortunately.
Open issues
While writing the schema, I noticed some quirks in our JSON schema:
assetsForLinking
toassets_for_linking
in JSON #2037prefer-static
toprefer_static
in JSON #2038input.config.code.android.target_ndk_api
should be required in JSON #2039This PR documents the current state, rather than to fix the issues in the same PR. (The first two require a breaking change and the last one requires a refactoring splitting syntactic and semantic errors.)
This PR is a step in the direction of: