Skip to content

types: narrow code reply type based on schema#5380

Merged
mcollina merged 2 commits intofastify:mainfrom
hpx7:hp/narrow-reply-type-from-schema
Apr 3, 2024
Merged

types: narrow code reply type based on schema#5380
mcollina merged 2 commits intofastify:mainfrom
hpx7:hp/narrow-reply-type-from-schema

Conversation

@hpx7
Copy link
Copy Markdown
Contributor

@hpx7 hpx7 commented Mar 31, 2024 •
edited
Loading

Closes #5378

Checklist

@hpx7 hpx7 mentioned this pull request Mar 31, 2024
Closed
2 tasks
climba03003
climba03003 previously requested changes Apr 1, 2024
View reviewed changes
@climba03003
Copy link
Copy Markdown
Member

climba03003 commented Apr 1, 2024

For others who would like to review this PR. We have two direction here

  1. Take a small step first and accept the incomplete of status code type-safe.
    That means 2xx or default remain no type-safe and exact status code would beneficial.

  2. Take a complete step and not accept a partial solution.

mcollina
mcollina reviewed Apr 1, 2024
View reviewed changes
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.

I think this is an improvement worth doing.

@climba03003 climba03003 dismissed their stale review April 1, 2024 08:22

Option 1 - Baby step first

@hpx7 hpx7 changed the title Narrow code reply type based on schema types: narrow code reply type based on schema Apr 1, 2024
@hpx7
Copy link
Copy Markdown
Contributor Author

hpx7 commented Apr 2, 2024

@mcollina could you take another look?

@climba03003 climba03003 force-pushed the hp/narrow-reply-type-from-schema branch from 6d91e51 to 2541e57 Compare April 3, 2024 03:23
@climba03003
Copy link
Copy Markdown
Member

climba03003 commented Apr 3, 2024

I have updated the branch by rebase.

@climba03003 climba03003 requested a review from a team April 3, 2024 03:24
@fox1t
Copy link
Copy Markdown
Member

fox1t commented Apr 3, 2024

I agree that at least fixing the 1. should be a tangible improvement anyway.

mcollina
mcollina approved these changes Apr 3, 2024
View reviewed changes
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

@mcollina mcollina merged commit c90ac14 into fastify:main Apr 3, 2024
@hpx7 hpx7 deleted the hp/narrow-reply-type-from-schema branch April 3, 2024 17:54
@urbankrepel urbankrepel mentioned this pull request May 28, 2024
Merged
@Viorel1989 Viorel1989 mentioned this pull request May 28, 2024
Merged
@raniellimontagna raniellimontagna mentioned this pull request May 28, 2024
Closed
@denbon05 denbon05 mentioned this pull request May 28, 2024
Merged
@X-oss-byte X-oss-byte mentioned this pull request May 29, 2024
Open
@JamieVangeysel JamieVangeysel mentioned this pull request May 29, 2024
Closed
@bryopsida bryopsida mentioned this pull request May 29, 2024
Merged
This was referenced May 30, 2024
Closed
Open
This was referenced Jun 1, 2024
Closed
Closed
@lholmquist lholmquist mentioned this pull request Jun 5, 2024
Closed
@JamieVangeysel JamieVangeysel mentioned this pull request Jul 26, 2024
Closed
@JamieVangeysel JamieVangeysel mentioned this pull request Aug 2, 2024
Closed
@NicolasTOISON NicolasTOISON mentioned this pull request Aug 7, 2024
Merged
@JamieVangeysel JamieVangeysel mentioned this pull request Aug 11, 2024
Closed
This was referenced Aug 21, 2024
Open
Open
Open
Open
Open
Open
Open
@Jamesp918 Jamesp918 mentioned this pull request Aug 24, 2024
Open
@JamieVangeysel JamieVangeysel mentioned this pull request Aug 25, 2024
Closed
This was referenced Aug 28, 2024
Open
Open
This was referenced Sep 3, 2024
Closed
Closed
This was referenced Sep 8, 2024
Open
Open
@JamieVangeysel JamieVangeysel mentioned this pull request Sep 10, 2024
Closed
@WesleyR10 WesleyR10 mentioned this pull request Sep 12, 2024
Open
@JamieVangeysel JamieVangeysel mentioned this pull request Sep 18, 2024
Closed
@Jamesp918 Jamesp918 mentioned this pull request Sep 18, 2024
Open
@JamieVangeysel JamieVangeysel mentioned this pull request Sep 24, 2024
Closed
@Jamesp918 Jamesp918 mentioned this pull request Sep 25, 2024
Open
@JamieVangeysel JamieVangeysel mentioned this pull request Sep 27, 2024
Closed
@Jamesp918 Jamesp918 mentioned this pull request Oct 15, 2024
Open
@JamieVangeysel JamieVangeysel mentioned this pull request Oct 27, 2024
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2025

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 Apr 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mcollina mcollina mcollina approved these changes

@fox1t fox1t fox1t approved these changes

@climba03003 climba03003 climba03003 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Type narrow on status().send() chains using Type-Providers schema

4 participants

@hpx7 @climba03003 @fox1t @mcollina