Skip to content

Docs/fluent schema#1485

Merged
mcollina merged 6 commits intofastify:masterfrom
Eomm:docs/fluent-schema
Mar 5, 2019
Merged

Docs/fluent schema#1485
mcollina merged 6 commits intofastify:masterfrom
Eomm:docs/fluent-schema

Conversation

@Eomm
Copy link
Copy Markdown
Member

@Eomm Eomm commented Feb 25, 2019

Closes #1329
I open this PR to get some feedback for this document and what examples I could add more.

Note that this line:

const bodyJsonSchema = { ...S.object().valueOf(), vacation: 'sharedAddress#' }

is necessary because the JSON schema standard (and fluent-schema too) doesn't provide support for a JSON like this:

{ "my-key": "sharedSchemaId#" }

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

@mcollina
Copy link
Copy Markdown
Member

I truly don't like:

const bodyJsonSchema = { ...S.object().valueOf(), vacation: 'sharedAddress#' }

Is there a way around this? The reason why this is needed must be in the docs.

Can you add some unit tests for the examples?

@Eomm
Copy link
Copy Markdown
Member Author

Eomm commented Feb 26, 2019

Is there a way around this?

It could be changed to a simply JSON:

{
    type: 'object',
    properties: {
      vacation: 'sharedAddress#'
    }
}

The reason why this is needed must be in the docs.

Could I add a reference to Validation-and-Serialization.md?

Can you add some unit tests for the examples?

Sure

@mcollina
Copy link
Copy Markdown
Member

It could be changed to a simply JSON:

THat's better yes.

Could I add a reference to Validation-and-Serialization.md?

yes of course.

@Eomm
Copy link
Copy Markdown
Member Author

Eomm commented Feb 26, 2019

@mcollina It seems that fluent-schema doesn't work with node 6. here the output
I will open an issue to fastify-schema to notify for this behaviour.

What do you suggest to solve?

@mcollina
Copy link
Copy Markdown
Member

skip the tests on Node 6.

@Eomm
Copy link
Copy Markdown
Member Author

Eomm commented Feb 26, 2019

skip the tests on Node 6.

Simple but effective 😁
I didn't saw this technique before 👏

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@aboutlo aboutlo left a comment

Choose a reason for hiding this comment

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

Good stuff here @Eomm! A couple of comments

Comment thread docs/Fluent-Schema.md Outdated
Comment thread docs/Fluent-Schema.md Outdated
Copy link
Copy Markdown
Contributor

@aboutlo aboutlo left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina mcollina merged commit 29a77b9 into fastify:master Mar 5, 2019
@Eomm Eomm deleted the docs/fluent-schema branch March 5, 2019 11:44
@delvedor delvedor added documentation Improvements or additions to documentation test Issue or pr related to our testing infrastructure. labels Mar 6, 2019
@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Improvements or additions to documentation test Issue or pr related to our testing infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants