Skip to content
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

Docs/Add standard json schema examples #1355

Merged
merged 5 commits into from
Feb 23, 2019
Merged

Conversation

Eomm
Copy link
Member

@Eomm Eomm commented Dec 26, 2018

Checklist

  • documentation is changed or added
  • commit message and code follows Code of conduct

Hi,
I'm opening this PR to get some feedback about this changing for issue #1329 .

I have written a "big" example with fluent schema because I think that put many little examples could be too broad and more confusing.

I have split the first example to view better the four main parameters of schema validation input (body, querystring, params and headers).

There are some questions I would like to expose you:

  • fluent-schema has started a big refactor for the v1 release, so I will update docs when it will be released
  • the definitions and $ref tags, as discussed in other issues like Encapsulate JSON schemas #1351 is supported by fluent-schema but not in fastify: should we implement that function or add a disclaimer in docs?

Let me know what do you think and what I should I do 😄
Thank you

PS: next days I will not have the PC so I'll continue with this PR at starts of January 💪

Copy link
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.

Good work, we will need to update this once Fluent-Schema goes v1.0.0. I’m +1 on landing this.

Regarding some of the other questions, I think you might want to open separate issues, and note that definitions and $ref are not supported. IMHO we could support those, but we should do in a separate PR (if you would like to do it, please send it over!)

docs/Validation-and-Serialization.md Outdated Show resolved Hide resolved
@Eomm
Copy link
Member Author

Eomm commented Dec 27, 2018

we could support those, but we should do in a separate PR (if you would like to do it, please send it over!)

Perfect, I'll do it!

@delvedor delvedor added the documentation Improvements or additions to documentation label Dec 27, 2018
@allevo
Copy link
Member

allevo commented Dec 27, 2018

I'm not so convinced to introduce this kind of suggestion into the core documentation: maybe a simple link to the repository or npm package is enough.

For example we introduce Joi with a very sort example here.

@mcollina
Copy link
Member

We have a lot of questions about json schema, it is not a common skill in web dev for some reasons. Providing a fluent API with good typings would go a long way in helping out newbies/beginners with Fastify.

I was thinking of putting it in the README, but it’s probably too early days for it.

@Eomm
Copy link
Member Author

Eomm commented Jan 4, 2019

Hi, I'm back 😄 Happy new year 🎉

Regarding this PR:
I saw that the refactor of fluent-schema booming

so, could be a good trade-off adds to docs more json-schema standard examples?
Then, with a future PR, could we add fluent schema when it will be released with the new API and fixes?

@mcollina
Copy link
Member

mcollina commented Jan 4, 2019

I'm good with that approach!

@stale
Copy link

stale bot commented Jan 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue or pr with more than 15 days of inactivity. label Jan 20, 2019
@Eomm
Copy link
Member Author

Eomm commented Jan 20, 2019

Hi stale bot 😋
I'm waiting for the review: as shared I reverted the fluent schema example (monitoring that repo for the new release) and added to this pr only the json schema standards

@stale stale bot removed the stale Issue or pr with more than 15 days of inactivity. label Jan 20, 2019
@aboutlo
Copy link
Contributor

aboutlo commented Feb 1, 2019

Meanwhile, fluent-schema v0.6.0 is out https://github.com/fastify/fluent-schema/releases/tag/v0.6.0 :)

@Eomm
Copy link
Member Author

Eomm commented Feb 5, 2019

I will rework on this PR soon: I'm thinking how to write a schema/table in order to show easily:

  • supported features
  • special features
  • examples
    for the validation and serialization with json-schema

@stale
Copy link

stale bot commented Feb 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue or pr with more than 15 days of inactivity. label Feb 20, 2019
@Eomm Eomm changed the title WIP Docs/json schema #1329 Docs/Add standard json schema examples Feb 21, 2019
@stale stale bot removed the stale Issue or pr with more than 15 days of inactivity. label Feb 21, 2019
@Eomm
Copy link
Member Author

Eomm commented Feb 21, 2019

Updated the title to match this #1355 (comment)

Copy link
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
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

LGTM

@delvedor delvedor merged commit 8202890 into fastify:master Feb 23, 2019
@github-actions
Copy link

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants