Skip to content

Generalize JSON schema handling code #14

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

Merged
merged 5 commits into from
Nov 5, 2020
Merged

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Nov 4, 2020

Previously, the JSON schema handling code was all under the libraryproperties package, and written specifically for use
in validating library.properties against the JSON schema. However, there will be be a need to work with schemas in other
contexts, most immediately for writing the schema tests, but also likely when additional schemas are added for the
other Arduino project configuration data files.

So it will be helpful to have the general purpose schema handling code
in a dedicated package, leaving only the library.properties-specific aspects of the code in the libraryproperties
package.


NOTE: the next PR will change to a different JSON schema package, which will involve changing almost all of the code in the schema package, as well as its API and removing the util package. So this PR is mostly just about moving the schema handling code to a dedicated package, as doing that in combination with the package change would not be atomic. I don't think it's necessary to spend a lot of time on a detailed analysis of the doomed code itself (which is mostly just being moved from one package to another anyway.

Previously, the JSON schema handling code was all under the libraryproperties package, and written specifically for use
in validating library.properties against the JSON schema. However, there will be be a need to work with schemas in other
contexts, most immediately for writitng the schema tests, but also likely when additional schemas are added for the
other Arduino project configuration data files.

So it will be helpful to have the general purpose schema handling code
in a dedicated package, leaving only the library.properties-specific aspects of the code in the libraryproperties
package.
@per1234 per1234 requested a review from silvanocerza November 4, 2020 08:25
Copy link
Contributor

@silvanocerza silvanocerza left a comment

Choose a reason for hiding this comment

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

Just some small nitpicks.

This allows the schemas path to be specified by the caller.
Rather than determining the schemas path once at initialization and storing it in a package variable, determine it on
demand when the getter function is called.
I don't foresee any need for this function outside the `schema` package, so it doesn't seem necessary to export it.
@per1234
Copy link
Contributor Author

per1234 commented Nov 5, 2020

I have pushed another commit to move util.PathURI() to the schema package: e47fcda

It turns out that I am still going to need to convert the schema file path to URI even after switching to the new JSON schema validation package. I don't foresee any need for this function outside the schema package, so I'm thinking it's best to just have it be a local function of that package.

@per1234 per1234 merged commit f1315c9 into main Nov 5, 2020
@per1234 per1234 deleted the per1234/generalize-schema-code branch November 5, 2020 13:57
@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Sep 29, 2021
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants