Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

docs: add v5 migration note for app.listen #1705

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

dpopp07
Copy link
Contributor

@dpopp07 dpopp07 commented Dec 3, 2024

Adds a note to the migrating-5 guide alerting users that error events received by a server will now cause the callback argument in app.listen to be invoked. In Express 4, this was not the case and errors would be thrown.

Resolves expressjs/express#6191

Any feedback, rewording, etc. is welcome!

Copy link

netlify bot commented Dec 3, 2024

Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit a042fc7
🔍 Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/67507a8bd28b4a0008bc71eb
😎 Deploy Preview https://deploy-preview-1705--expressjscom-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bjohansebas bjohansebas requested review from wesleytodd and a team December 3, 2024 17:26
Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

Great

@dpopp07 dpopp07 force-pushed the dp/app-listen-change-v5 branch from ae63d87 to 40e9db5 Compare December 4, 2024 15:47
Adds a note to the `migrating-5` guide alerting users that error events
received by a server will now cause the callback argument in `app.listen` to
be invoked. In Express 4, this was not the case and errors would be thrown.

Resolves expressjs/express#6191

Signed-off-by: Dustin Popp <dustinpopp@ibm.com>
Co-authored-by: M. Heide <66078329+heidemn-faro@users.noreply.github.com>
@dpopp07 dpopp07 force-pushed the dp/app-listen-change-v5 branch from 40e9db5 to a042fc7 Compare December 4, 2024 15:51
@dpopp07
Copy link
Contributor Author

dpopp07 commented Dec 4, 2024

@bjohansebas @carlosstenzel thanks for reviewing!

I cleaned up the linter errors. Is there anything I can do to fix the "Check Translation" build step failure?

@bjohansebas
Copy link
Member

Hi @dpopp07, don't worry about the Check Translation workflow, it will always fail for PRs coming from a fork.

I think this workflow should be removed

@dpopp07
Copy link
Contributor Author

dpopp07 commented Dec 9, 2024

@bjohansebas Great - I thought that might be the case but wanted to confirm you didn't need anything else from me. Thanks!

Copy link
Member

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

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

Great work @dpopp07! LGTM!

@UlisesGascon UlisesGascon merged commit 4f881e5 into expressjs:gh-pages Dec 20, 2024
9 of 10 checks passed
@dpopp07 dpopp07 deleted the dp/app-listen-change-v5 branch December 20, 2024 15:28
# for free to join this conversation on GitHub. Already have an account? # to comment