Skip to content
This repository was archived by the owner on Sep 12, 2018. It is now read-only.

Allow instants to pass through schema validation. #481

Merged
rnewman merged 2 commits into from
Jun 16, 2017
Merged

Allow instants to pass through schema validation. #481

rnewman merged 2 commits into from
Jun 16, 2017

Conversation

@rnewman
Copy link
Collaborator

@rnewman rnewman commented Jun 16, 2017

We missed a case. Hooray placeholders!

@rnewman rnewman self-assigned this Jun 16, 2017
@rnewman rnewman requested a review from ncalexan June 16, 2017 01:41
@rnewman rnewman mentioned this pull request Jun 16, 2017
Closed
fluffyemily
fluffyemily approved these changes Jun 16, 2017
View reviewed changes
Copy link
Contributor

@fluffyemily fluffyemily left a comment

Choose a reason for hiding this comment

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

I'd made this change already in the CLI tool, but probably best to get it in.

Channeling @ncalexan's comment when I did this - we should also expand the default case to cover all remaining cases so that we don't miss a case next time!

@rnewman
Copy link
Collaborator Author

rnewman commented Jun 16, 2017

Channeling @ncalexan's comment when I did this - we should also expand the default case to cover all remaining cases so that we don't miss a case next time!

I thought about doing that… but don't we have a product here of all ValueTypes and all TypedValues? That's 56 more cases. We could choose to only do ValueType or TypedValue thoroughly…

@rnewman
Copy link
Collaborator Author

rnewman commented Jun 16, 2017

@fluffyemily ^ howzat?

fluffyemily
fluffyemily approved these changes Jun 16, 2017
View reviewed changes
Copy link
Contributor

@fluffyemily fluffyemily left a comment

Choose a reason for hiding this comment

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

Yup, that's pretty much what I did :)

aside: I've not seen the vt @ ... syntax before. I shall look it up.

@rnewman rnewman merged commit ea0e9d4 into master Jun 16, 2017
@rnewman rnewman deleted the rnewman/fix-instants-schema branch June 16, 2017 16:15
RDR8 pushed a commit to RDR8/mentat that referenced this pull request Mar 12, 2018
@rnewman @RDR8
Allow instants to pass through schema validation. (mozilla#481) r=flu…
b81b2dd 
…ffyemily

* Allow instants to pass through schema validation.
* Expand cases in SchemaTypeChecking to catch enum bugs.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ncalexan ncalexan Awaiting requested review from ncalexan

1 more reviewer

@fluffyemily fluffyemily fluffyemily approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rnewman rnewman

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rnewman @fluffyemily