Skip to content

Support JSON Schema in determineScalarType#2

Open
P0lip wants to merge 1 commit into
masterfrom
feat/yaml-json-schema
Open

Support JSON Schema in determineScalarType#2
P0lip wants to merge 1 commit into
masterfrom
feat/yaml-json-schema

Conversation

@P0lip

@P0lip P0lip commented Jan 21, 2020

Copy link
Copy Markdown

@P0lip P0lip added the enhancement New feature or request label Jan 21, 2020
@P0lip P0lip self-assigned this Jan 21, 2020
@P0lip

P0lip commented Jan 21, 2020

Copy link
Copy Markdown
Author

@nulltoken what do you think?
Using JSON Schema is more strict, therefore I expect the change might be painful.

@nulltoken

Copy link
Copy Markdown

@nulltoken what do you think?

@P0lip I like it!

Using JSON Schema is more strict, therefore I expect the change might be painful.

Maybe prep up a migration strategy with an optional switch for next major version set to 'Core' by default and warning users that they should test their own use cases with the 'JsonSchema' value (and report weird bugs). And then set 'JsonSchema' as the default for vn+1?

Few additional thoughts:

  • Do we also support octal and hex representations of an int?
  • I believe the Spectral doco should eventually point out to some sections of the YAML specs in order to make it clear what's supported/expected and what's not.

Comment thread src/scalarInference.ts
case DetermineScalarSchema.Core:
return determineScalarTypeForCoreSchema(node);
case DetermineScalarSchema.Json:
return determineScalarTypeForJsonSchema(node);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

throw on default to protect from future mistakes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Falsely Identifying Strings as Octals

2 participants