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

feat(remix-vercel): show deprecation warning on usage #5964

Conversation

MichaelDeBoey
Copy link
Member

As @mcansh mentioned in #5734 (comment)

@changeset-bot
Copy link

changeset-bot bot commented Mar 31, 2023

⚠️ No Changeset found

Latest commit: c14e8b2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch 2 times, most recently from 58c1465 to ddda79c Compare March 31, 2023 23:31
@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch 3 times, most recently from 421007e to cb4dda8 Compare April 14, 2023 16:17
@mcansh
Copy link
Collaborator

mcansh commented Apr 17, 2023

we're still deciding on what to do with our vercel adapter at the moment, so going to hold off on this one for now

@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch from cb4dda8 to 63754f9 Compare April 19, 2023 20:32
@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch from 63754f9 to ee4222b Compare April 24, 2023 00:58
Copy link
Collaborator

@mcansh mcansh left a comment

Choose a reason for hiding this comment

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

instead of warning on usage, can we add this message to the top of the vercel template readme?

@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented Apr 25, 2023

@mcansh We could do both?
What I had also in mind was adding the deprecation warning to the npm package itself

@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch 2 times, most recently from d0e81c6 to 14acc70 Compare May 1, 2023 21:05
@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch 2 times, most recently from e83b8cb to aed55cf Compare May 6, 2023 17:23
@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch from aed55cf to de9cea6 Compare May 15, 2023 20:28
@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch 2 times, most recently from 58ae2a7 to 49ae1e8 Compare May 28, 2023 23:16
@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch from 49ae1e8 to 888213c Compare June 6, 2023 21:22
Copy link
Contributor

@brophdawg11 brophdawg11 left a comment

Choose a reason for hiding this comment

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

Confirmed with @mjackson we were good to deprecate this entirely in v2

@MichaelDeBoey MichaelDeBoey force-pushed the show-deprecation-warning-on-vercel-adapter-usage branch from 888213c to c14e8b2 Compare June 13, 2023 20:06
@brophdawg11 brophdawg11 merged commit 320f158 into remix-run:dev Jun 13, 2023
8 of 9 checks passed
@MichaelDeBoey MichaelDeBoey deleted the show-deprecation-warning-on-vercel-adapter-usage branch June 13, 2023 22:02
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-eb06147-20230614 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.17.1-pre.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@brophdawg11
Copy link
Contributor

Hm, we must have a bug in our bot logic - this is not included in 1.17.1-pre.1 and should land in 1.18.0

https://github.com/remix-run/remix/compare/remix@1.17.0...remix@1.17.1-pre.1

@mcansh
Copy link
Collaborator

mcansh commented Jun 14, 2023

Hm, we must have a bug in our bot logic - this is not included in 1.17.1-pre.1 and should land in 1.18.0

https://github.com/remix-run/remix/compare/remix@1.17.0...remix@1.17.1-pre.1

yeah that’s odd… it gets the commits via git log --pretty=format:%H v0.0.0-nightly-eb06147-20230614...remix@1.17.1-pre.1 ./packages, but seems like github actions got a different list 🤔

indeed not here, but somehow got into the list in ci

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-ad9adee-20230615 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-12440f3-20230616 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@TrySound
Copy link
Contributor

Seems like with vercel maintaining a fork of remix we getting less usable integration. As every remix bump is blocked until vercel decides to rebase their fork.

image image

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.18.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.18.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants