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.

Sign up for GitHub

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

Jump to bottom

feat: upper case the name of the endpoints #7783

Merged
merged 8 commits into from
Jul 26, 2023
Merged

feat: upper case the name of the endpoints #7783

merged 8 commits into from
Jul 26, 2023

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Jul 24, 2023 •
edited
Loading

Changes

Note: Astro 3.0 breaking change

Note: PR to next branch

Now the endpoints must be uppercase, and we don't support anymore the del function, but it will be DELETE.

Testing

Updated all the tests that use the old endpoints. Current tests should all pass.

Docs

/cc @withastro/maintainers-docs for feedback!

@changeset-bot
Copy link

changeset-bot bot commented Jul 24, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 548a193

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Jul 24, 2023
@natemoo-re
Copy link
Member

natemoo-re commented Jul 24, 2023 •
edited
Loading

Just double checking SvelteKit's endpoints and making sure we support all the same things. Should we add OPTIONS and HEAD?

  • GET
  • POST
  • PATCH
  • PUT
  • DELETE
  • OPTIONS
  • HEAD

Also is this a hard deprecation (must migrate) or soft deprecation (should migrate, but can leave the code as-is until later)

@ematipico
Copy link
Member Author

ematipico commented Jul 24, 2023 •
edited
Loading

Should we add OPTIONS and HEAD?

In this PR, I don't think we should. It's a new feature that we don't have. It can be implemented without a breaking change.

Also is this a hard deprecation (must migrate) or soft deprecation (should migrate, but can leave the code as-is until later)

Yeah it probably makes sense to deprecate the old ones. I will push the change

@ematipico ematipico changed the title feat: capitalize the name of the endpoints feat: upper case the name of the endpoints Jul 24, 2023
@ematipico ematipico force-pushed the feat/capitalized-endpoints branch 2 times, most recently from fd758f6 to 1c1e1f6 Compare July 25, 2023 07:23
yanthomasdev
yanthomasdev reviewed Jul 25, 2023
View reviewed changes
Copy link
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

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

Thanks @ematipico, added a suggestion to improve the changeset 🙌

.changeset/twelve-coats-rush.md Outdated Show resolved Hide resolved
delucis
delucis reviewed Jul 25, 2023
View reviewed changes
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks for this @ematipico!

A few quick comments, mainly focused on DX stuff like the CHANGELOG & deprecation logging.

.changeset/twelve-coats-rush.md Outdated Show resolved Hide resolved
packages/astro/src/runtime/server/endpoint.ts Outdated Show resolved Hide resolved
packages/astro/src/runtime/server/endpoint.ts Show resolved Hide resolved
.changeset/twelve-coats-rush.md Outdated Show resolved Hide resolved
ematipico and others added 7 commits July 25, 2023 16:20
@ematipico
feat: capitalize the name of the endpoints
d10a040
@ematipico
chore: deprecated current endpoints
0f0774e
@ematipico @bluwy
Update packages/astro/src/runtime/server/endpoint.ts
9b0adac 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
@ematipico @yanthomasdev
Update .changeset/twelve-coats-rush.md
d4fb70a 
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
@ematipico @delucis
Update .changeset/twelve-coats-rush.md
468b386 
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
@ematipico @delucis
Update .changeset/twelve-coats-rush.md
636804a 
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
@ematipico
feedback and regression
9570695
@ematipico ematipico merged commit 1be3605 into next Jul 26, 2023
@ematipico ematipico deleted the feat/capitalized-endpoints branch July 26, 2023 07:53
ematipico added a commit that referenced this pull request Jul 26, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
d8ae80b 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Jul 26, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
b1e8878 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Jul 27, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
21f77c4 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Jul 31, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
bc317b2 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Aug 1, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
c8435c3 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Aug 1, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
a6e5135 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Aug 3, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
78de801 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
ematipico added a commit that referenced this pull request Aug 8, 2023
@ematipico @bluwy
@yanthomasdev @delucis
feat: upper case the name of the endpoints (#7783)
0f06255 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanthomasdev yanthomasdev yanthomasdev left review comments

@delucis delucis delucis left review comments

@bluwy bluwy bluwy approved these changes

Assignees
No one assigned
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ematipico @natemoo-re @delucis @bluwy @yanthomasdev