Skip to content

feat: add mkcalendar & Report to body methods in next#5444

Closed
keyservlad wants to merge 7 commits intofastify:nextfrom
keyservlad:add-mkcalendar-report-next
Closed

feat: add mkcalendar & Report to body methods in next#5444
keyservlad wants to merge 7 commits intofastify:nextfrom
keyservlad:add-mkcalendar-report-next

Conversation

@keyservlad
Copy link
Contributor

Checklist

For context see #5439

Comment on lines +28 to +30
'SEARCH',
'MKCALENDAR',
'REPORT'
Copy link
Member

@gurgunday gurgunday May 3, 2024

Choose a reason for hiding this comment

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

This is normally in alphabetical order

Also just realized 'MKCOL' should be above 'MOVE' for the same reason, would be great if you could move it too

@gurgunday
Copy link
Member

The rest of the changes we can pull from main I think, but maybe we should add their shorthands here as well

Like this: #4409

@keyservlad
Copy link
Contributor Author

keyservlad commented May 3, 2024

I merged main and added the route shorthand methods.
Still getting the route-shorthand test failing tho ahah ;D

Edit: Fixed it

@gurgunday
Copy link
Member

gurgunday commented May 3, 2024

yeah looks great, just one issue, because we use squash merge, I think we should merge main to next first, and merge this after that, otherwise all these merge changes will end up being conflicts

edit: or maybe not

gurgunday added 2 commits May 4, 2024 15:37
Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
gurgunday
gurgunday previously approved these changes May 4, 2024
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

lgtm

Oops, wrong place 😄

So we had to manually merge to next and had to include the changes of this PR, thanks anyway for the PR!

@keyservlad
Copy link
Contributor Author

@gurgunday Thank you guys very much for your work. I noticed you didn't include the route shorthand changes for the new methods. If you ever want to implement them, you can find the changes needed in this PR.

Here are the files to change :
docs/Reference/Routes.md

test/types/route.test-d.ts
instance.d.ts

@gurgunday
Copy link
Member

Good spot, the shorthands were added, but you are right that the types weren't, opening the PR again to not forget

@gurgunday gurgunday reopened this May 6, 2024
@keyservlad
Copy link
Contributor Author

I will try to rebase and get rid of the commits that come from the merge if that's ok with you

@gurgunday
Copy link
Member

Yeah but I’d say let’s wait for the merge to main of v5 first, like @jsumners said, or maybe he’ll add it manually

@mcollina mcollina deleted the branch fastify:next May 7, 2024 13:07
@mcollina mcollina closed this May 7, 2024
@gurgunday
Copy link
Member

Again, sorry for the trouble, can you open the type PR again? It can be a new PR to make things easier, and it should target main now

@github-actions
Copy link

github-actions bot commented May 8, 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 May 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants