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

bump v5 router to beta.1 #4321

Merged
merged 1 commit into from
Dec 17, 2021
Merged

bump v5 router to beta.1 #4321

merged 1 commit into from
Dec 17, 2021

Conversation

jonchurch
Copy link
Member

I went to test the new path-to-regexp behavior in 5.0.0-alpha.8 and realized that it relies on router@alpha.1, which doesn't have the new path-to-regexp behavior that's planned for v5.

Not sure if this is intentional or not, so I wanted to open this PR as a reminder.

I'm writing a blog post for work about changes in v5 and wanted folks to be able to try this part out.

@jonchurch jonchurch added the 5.x label Jun 18, 2020
@dougwilson
Copy link
Contributor

I wanted to get that PR you created in the router module into the next 5 release, which is the hold up. It was all set, I though, until all those review comments came in. I pinged recently on if it was actually ready or not and it sounds like there are still some outstanding things you need to fix in the PR in order to land @jonchurch 👍

@dougwilson dougwilson added this to the 5.0 milestone Jun 18, 2020
@dougwilson
Copy link
Contributor

I have a local pending change that I was going to PR as soon as that was open. If you want to pursue it here, just needs the existing examples and tests updated for the new routing syntax (so CI passes) and since we also keep some of the key pieces of tests in this module as well (like body parser, serve static, and router), you'll want to also copy in the tests for the new features the router is now bringing in. I hope that helps if you want to take up helping push this PR forward 👍

@jonchurch
Copy link
Member Author

Ah, ty doug. I committed wes' doc suggestions in the other PR.

I can pick this up, but might not have time until next week.

@jonchurch
Copy link
Member Author

I'm behind on my notifications and need to chew through those, I'm sure there's some other things in there I'm missing like that PR.

@dougwilson
Copy link
Contributor

It's cool, there is no rush :) I was more trying to understand what your plans were to coordinate. If you're not even planning to pick up for the next couple days even, I will try to push up my commits into your PR here to give you a boost when you get to it, if that's OK with you. Don't wait on me either, but I should still have that git folder from my old machine to push up 🥳

@jonchurch
Copy link
Member Author

Sounds good, push away! I opened this mostly as a slightly more useful comment than opening an issue.

@thernstig
Copy link

@dougwilson is this the last PR for #4535 ? It seems @jonchurch has not been active on Github for over a year, so it is fair to say he won't be able to fix this. You have the deepest knowledge of Express and are an active coder, is there a chance you could take over and fix these last things to get 5.0 released?

@thernstig
Copy link

It's cool, there is no rush :) I was more trying to understand what your plans were to coordinate. If you're not even planning to pick up for the next couple days even, I will try to push up my commits into your PR here to give you a boost when you get to it, if that's OK with you. Don't wait on me either, but I should still have that git folder from my old machine to push up 🥳

In reference to #4535 (comment) I think it is fair to say that you @jonchurch gave you the go-ahead on this. But since @jonchurch is back to fix this now that is great 😃

@thernstig
Copy link

@jonchurch is there any chance you could give it a go to finalize this 😄 It is the only outstanding issue for Express 5.0 to be released: #4535 (comment) - the community is loving you for fixing this ❤️

@xjamundx
Copy link

What's the status of this?

@dougwilson
Copy link
Contributor

I went ahead and updated the tests for changed functionality and added tests for new features, so this is ready to land in 5.0

@thernstig
Copy link

@dougwilson does this mean you could consider releasing Express 5 once expressjs/expressjs.com#1313 is fixed? 😃

@nfantone
Copy link

nfantone commented Jan 3, 2022

I'm confused. Once pillarjs/router#102 is merged, a new (beta?) version of router would need to be cut and express would need to be updated again.

Also changes in 1574925 were already part of #4752, which was put on hold and marked as "Draft" until the above takes place, but I see no mention of this.

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.

5 participants