-
Notifications
You must be signed in to change notification settings - Fork 206
chore(deps): drop support for Express 2.x and use uid-safe for async helpers tracking #225
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
base: 5.0
Are you sure you want to change the base?
Conversation
Hey @mfdebian, thanks for your interest in contributing to this package. The recommended and best approach is always to open separate PRs, since a single change can block the entire PR or make the review process complicated if it turns into a huge PR |
Thank you @bjohansebas! Should I rename this PR and just use it to merge the cherry-picked commits? |
Yes, that’s the best. Any other changes you want to make should go in separate and dedicated PRs. |
Done, thank you @bjohansebas! |
I'm curious as to why those commits are being brought in, given that they are breaking changes and, according to SemVer, they should be released in a new major version |
Yeah you're right, sorry, I misunderstood Ulises comments on #203 , the idea is to prepare a v5.0 release and I'm trying to help on that. Maybe we can move this PR to point to a Sorry I took so long to respond/take action, I was on call for the FIFA Club World Cup and it ended just now so now I can take on this 🚀 Let me know your thoughts! And thank you 🙌 ! |
Take a look at the process for how releases work within express. while it's focused solely on express, it's the same way releases are handled at the project level (i already have a PR updating it expressjs/discussions#387 to make this a global process, but it works the same). This process will be carried out by the captain of this package. The changes from this PR are already where they need to be, which is the 5.0 branch, so there’s no need to change the target branch as you suggested. I think the next steps you can take are to open PRs for the features you want to add, update support for the latest Node.js versions (both on master and the 5.0 branch), remove support for older versions, etc. Please keep each change in separate PRs. |
My bad on the work definition, here is a better approach (I hope) as now I created a clean and updated version of the branch Some ideas:
... wdyt @mfdebian ? |
I'll start with this minor changes and build from here @UlisesGascon @bjohansebas tell me if you think this is the correct approach or not.
For now I'll leave #203 open