-
-
Notifications
You must be signed in to change notification settings - Fork 12
Add permissive and strict library.properties JSON schemas #25
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
Conversation
5edeafe
to
bed28de
Compare
I started the arduino-check project with the simplistic vision of it being a tool used to enforce the Arduino Sketch/Library/Platform/Package Index Specifications. However this approach neglects some important use cases: - Library Manager index management. Specification violations don't always result in breakage, so library authors may resent a strict requirement that all libraries added to the index be specification-compliant. More significantly, large numbers of non-compliant libraries are already in the index, so a change to blocking non-specification compliant releases would likely be very disruptive. For this reason, a "permissive" subset of the specification is needed. - Advocating best practices. Although not mandated by the specification, some practices result in an improved experience for the user, and thus should be advocated by Arduino. It would be unreasonable to enforce these practices on unwilling users, but they can be advocated for with warnings and by offering willing users an option to use the tool in a "pedantic" mode. For this reason, a "strict" superset of the specification is needed. JSON schemas offer a standardized method of defining a data format. This form of information can be easily used by consumers other than the arduino-check tool. So I think it's the best approach to define these compliance levels in schemas, rather than directly in Go code. Significant components of the schemas are identical. To avoid the need to maintain duplicate content, it's important to use referencing. While working on this, I found that it it was confusing to work with a separate file for the definition of each compliance level of each document property, especially when considering how changing one schema affects all the schemas that reference it, and the schemas that reference those schemas... I settled on the approach of using a monolithic definitions schema, resulting in the parent schema being nearly identical collection of references to that file, which are unlikely to need to be updated at all in the future, even when modifications are made to the definitions schema. With the idea that schemas for the other Arduino project configuration files will be created in the future to expand the scope of arduino-check, I put the definitions that can be used by other schemas in a dedicated definitions schema, from which they can be referenced by additional schemas.
bed28de
to
2d758b0
Compare
// To purchase a commercial license, send an email to license@arduino.cc. | ||
|
||
// Package schemas contains tests for the JSON schemas. | ||
package schemas |
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.
I'm not sure whether I took the right approach by creating this empty package in order to have the JSON schemas tests.
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.
What are you actually testing here? Looking at the tests I'd probably put them under the libraryproperties
package, but I might be wrong.
I think it's kinda strange creating an empty package for testing by the way. Have you tried deleting this file and leave only its related _test.go
? Not sure if it works really.
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.
What are you actually testing here?
The configuration of the JSON schemas themselves, the tests in etc/schemas/test/librarypropertiesschemas_test.go
are not targeted at testing the Go code.
I'd probably put them under the libraryproperties package
OK, if you think that's the better place, I'll move them there: 3a0bf88
Have you tried deleting this file and leave only its related _test.go?
That's what I did originally, but it causes task go:test-unit
to fail:
https://github.com/per1234/arduino-check/runs/1414468912?check_suite_focus=true#step:6:12
go build github.com/arduino/arduino-check/etc/schemas/test: no non-test Go files in /home/runner/work/arduino-check/arduino-check/etc/schemas/test
? github.com/arduino/arduino-check [no test files]
? github.com/arduino/arduino-check/check [no test files]
? github.com/arduino/arduino-check/check/checkconfigurations [no test files]
? github.com/arduino/arduino-check/check/checkdata [no test files]
FAIL github.com/arduino/arduino-check/check/checkdata/schema [build failed]
I did some research and found the fix of adding an empty go file:
https://github.com/golang/go/issues/22409#issuecomment-382985756
This can be solved with a simple workaround of adding any dummy file with a non _test suffix in the same package. Just have a ignore.go file with a one-liner package tests, and everything works.
https://github.com/golang/go/issues/8279#issuecomment-66096402
Adding a dummy.go file (with only "package " line) works
https://stackoverflow.com/a/47025370/7059512 / https://stackoverflow.com/a/62394096/7059512
Add doc.go file to your tests pkg and call it package tests in both files
I started the arduino-check project with the simplistic vision of it being a tool used to enforce the Arduino Sketch/Library/Platform/Package Index Specifications. However this approach neglects some important use cases:
Library Manager index management. Specification violations don't always result in breakage, so library authors may resent a strict requirement that all libraries added to the index be specification-compliant. More significantly, large numbers of non-compliant libraries are already in the index, so a change to blocking non-specification compliant releases would likely be very disruptive. For this reason, a "permissive" subset of the specification is needed.
Advocating best practices. Although not mandated by the specification, some practices result in an improved experience for the user, and thus should be advocated by Arduino. It would be unreasonable to enforce these practices on unwilling users, but they can be advocated for with warnings and by offering willing users an option to use the tool in a "pedantic" mode. For this reason, a "strict" superset of the specification is needed.
JSON schemas offer a standardized method of defining a data format. This form of information can be easily used by consumers other than the arduino-check tool. So I think it's the best approach to define these compliance levels in schemas, rather than directly in Go code.
Significant components of the schemas are identical. To avoid the need to maintain duplicate content, it's important to use referencing. While working on this, I found that it it was confusing to work with a separate file for the definition of each compliance level of each document property, especially when considering how changing one schema affects all the schemas that reference it, and the schemas that reference those schemas... I settled on the approach of using a monolithic definitions schema, resulting in the parent schema being nearly identical collection of references to that file, which are unlikely to need to be updated at all in the future, even when modifications are made to the definitions schema.
With the idea that schemas for the other Arduino project configuration files will be created in the future to expand the scope of arduino-check, I put the definitions that can be used by other schemas in a dedicated definitions schema, from which they can be referenced by additional schemas.