-
Notifications
You must be signed in to change notification settings - Fork 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
Adds impl of IonSchemaReaderV2_0 and ion-schema-tests for readers #259
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #259 +/- ##
============================================
+ Coverage 82.02% 82.75% +0.72%
- Complexity 717 791 +74
============================================
Files 104 116 +12
Lines 2632 3016 +384
Branches 524 595 +71
============================================
+ Hits 2159 2496 +337
- Misses 350 363 +13
- Partials 123 157 +34
☔ View full report in Codecov by Sentry. |
private val typeReader = TypeReaderV2_0() | ||
|
||
override fun readSchema(document: List<IonValue>, failFast: Boolean): IonSchemaResult<SchemaDocument, List<ReadError>> { | ||
TODO() |
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.
🗺️ To be clear, this intentionally not a complete implementation yet (in order to keep PR sizes smaller).
@ExperimentalIonSchemaModel | ||
class ReaderTests { | ||
|
||
val unimplementedConstraints = listOf( |
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.
Would this be easier as a positive list rather than a negative one?
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 don't think so... this list is going to be shrinking with every PR I send, and will eventually be gone entirely.
val dynamicNodeTestCases = dg.mapNotNull { ion -> | ||
// The reader is a little more sophisticated than ISL for ISL, but it cannot detect problems with | ||
// duplicate type names or imports. | ||
if (ion !is IonStruct || !ion.islForIslCanValidate()) return@mapNotNull null |
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.
The islForIslCanValidate()
extension function feels strange to me? I'm getting unpleasant flashbacks of Ruby monkey-patching. I understand that Kotlin extensions are much better than monkey-patching etc. etc.
Looking at the definition of the method though, it makes sense... 🤷 . Nothing to do here.
Issue #, if available:
#256
Description of changes:
Adds initial, partial implementation of
TypeReaderV2_0
andIonSchemaReaderV2_0
, but more importantly, it sets up a test runner that can leverageion-schema-tests
for the reader implementation.Related PRs in ion-schema, ion-schema-tests, ion-schema-schemas:
None.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.