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

express 5.0.0 #100

Closed
gireeshpunathil opened this issue Feb 23, 2020 · 45 comments
Closed

express 5.0.0 #100

gireeshpunathil opened this issue Feb 23, 2020 · 45 comments
Assignees
Labels
discuss top priority Issues which the TC deem our current highest priorities for the project

Comments

@gireeshpunathil
Copy link

the original issue (PR expressjs/express#2237) that tracks this is locked up for probable unmanageable number of discussions and some of which off-topic? I can't find an active issue that tracks this, so opening one.

I request to release express 5.

  • how was the user feedback on 5.0.0-alpha.7? if it is reasonably positive, does it make sense to cut that into 5?
  • are there pending items in Release 5.0 express#2237 (comment) that are real blockers? can they be pushed to in 5.1?
  • can we have a TC meeting to discuss this - challenges, gaps, actionables and owners?

I am not an express commiter, but a staunch fan. I can commit the necessary bandwidth required for upskilling myself and contribute to realize this!

@gireeshpunathil gireeshpunathil self-assigned this Feb 23, 2020
@dougwilson
Copy link
Contributor

how was the user feedback on 5.0.0-alpha.7? if it is reasonably positive, does it make sense to cut that into 5?

There has generally not been any so far...

are there pending items in expressjs/express#2237 (comment) that are real blockers? can they be pushed to in 5.1?

Those are simply the steps left to do.

can we have a TC meeting to discuss this - challenges, gaps, actionables and owners?

Sur!. Please open an issue in the discussions repo for meeting requests 👍

@dougwilson
Copy link
Contributor

I think ultimately the last piece at this point for express 5 is to include the update for the new path-to-regexp which will also need a corresponding section created in http://expressjs.com/en/guide/migrating-5.html

I think the following are the tasks currently for the next 5 release (beta from current alpha):

  1. Get the express dep upgrades all rolled out into a 4.x patch release.
  2. Roll the new patch into the 5.0 branch
  3. Publish router beta with new route syntax and put in 5.0 branch
  4. Publish 5.0 beta
  5. Update migration guide on expressjs
  6. Update announcement to it being beta

From there, with the new route syntax, it would be useful if some real world apps were migrated to 5 before it was published as 5.0 due to the routing syntax changes. Not only to be prepared to answer incoming question and/or refine our guide, but also find issues.

@dougwilson dougwilson reopened this Feb 23, 2020
@dougwilson
Copy link
Contributor

(Sorry i didn't mean to close the issue after my first response)

@gireeshpunathil
Copy link
Author

@dougwilson - here is a request for TC sitting: #99 . I am good with any day, any time.

@dougwilson
Copy link
Contributor

So for some more things to think about here for planning, is that once 5.0 is released how long do we want to support 4.x? In the past it has been 1 year, but we should decide if that makes sense to us. With the changes, I think it is going to be some level of effort to upgrade a large app from 4 to 5, so it seems the longer we can make it, the nicer experience it would be.

Also we may want to comb over the issues in our deps (the ones in our orgs) as I forgot that some have major of their own tied into express 5. The router module was mentioned above already, but body-parser should be another and I think with the "mime" module suddenly archiving it's repo, serve-static may need to be another one as well (to include the switch from mime to mime-types).

This is just a though dump comment, so it's not well formed. More like notes for myself for the tc meeting but everyone can see before hand lol

@dougwilson dougwilson transferred this issue from expressjs/express Feb 23, 2020
@dougwilson dougwilson added discuss top priority Issues which the TC deem our current highest priorities for the project labels Feb 23, 2020
@gireeshpunathil
Copy link
Author

@dougwilson -

new path-to-regexp which will also need a corresponding section created in http://expressjs.com/en/guide/migrating-5.html

I would like to drive this to closure, any guidance on where to start or whom to contact?

  1. Publish 5.0 beta

Why do we want a beta release? We ran into extra-ordinarily long iterations of alphas, and as there wasn't any concerns from its users, why can't we go straight upto 5.0.0 GA? anyways, we shall discuss this in the TC!

I am stating my opinion here that, releasing express 5.0.0 (as GA, not as beta) is the best thing to do, for the ecosystem.

migration guide:

Who is / are the SMEs here? I would like to request that all the maintainers and triagers be educated on this topic - a couple of dedicated sessions if need be would be great. If you can share the info on the SME part, I can co-ordinate with them on this space.

how long do we want to support 4.x?

My take is that we defer it (taking a decision) to after the release of 5 and some insights from the field are gathered? Until then the standard 1 year applies, with a caveat of future amendments?

Also we may want to comb over the issues in our deps

May be candidates for 6, 7, 8... ?

@dougwilson
Copy link
Contributor

I would like to drive this to closure, any guidance on where to start or whom to contact?

The path-to-regexp update has mainly been driven by @wesleytodd

Why do we want a beta release? We ran into extra-ordinarily long iterations of alphas, and as there wasn't any concerns from its users, why can't we go straight upto 5.0.0 GA?

We can absolutely do that, but it means that we drop adding anything new at this point and release alpha.7 as the GA. That is your suggestion, correct?

Who is / are the SMEs here? I would like to request that all the maintainers and triagers be educated on this topic - a couple of dedicated sessions if need be would be great. If you can share the info on the SME part, I can co-ordinate with them on this space.

That is such a large question, I'm not even sure how to really answer it :) I mean, I would probably say in a general sense myself and @wesleytodd /shrug

May be candidates for 6, 7, 8... ?

IMO one thing I really like about Express, and pretty much all my coworkers, is the lack of constant breaking changes and majors. I would push back extremely strongly regarding this, unless you're talking about the far future. IMO I think we should not be releasing majors faster than Node.js itself, if even that. That means that if we release 5.0 today, then we're deferring everything not included into it until at least Feb 2021... If we want to do that, I think we should get a good idea on what exactly we are talking about here and the specifics around such a sudden change in direction that is so drastic.

@gireeshpunathil
Copy link
Author

@dougwilson -

We can absolutely do that, but it means that we drop adding anything new at this point and release alpha.7 as the GA. That is your suggestion, correct?

Yes.

That means that if we release 5.0 today, then we're deferring everything not included into it until at least Feb 2021

Generally agree if there are immediate takers of the delta features - but as you rightly said, we need to look at those items to get a better feel. But here is a consumer's perspective: the more features get added to a major, the tougher it becomes to test, assess the impact, and migrate. I am not saying a 6 months cadence is good for this project, but on the other hand, too long gaps between majors make it hard to migrate too?

I would push back extremely strongly regarding this

I guess this is where the real difference in opinion is, if at all - all others being just general sync up and ratification. How do we gather consensus on this? - express release cadence

  • deliberate in a separate issue
  • discuss in a dedicated meeting
  • gather user feedback

?

@gireeshpunathil
Copy link
Author

I opened https://github.com/expressjs/express/issues/4197 for item 3 above

@dougwilson
Copy link
Contributor

I opened expressjs/express#4197 for item 3 above

I'm confused here: why ask a question for feedback on what to do, but then don't even wait for it? Why even ask the question "How do we gather consensus on this?" if you're not going to wait for any response?

@gireeshpunathil
Copy link
Author

@dougwilson - I acknowledge your confusion. I left item 1 and 2 to hear from you while initiated item 3, as it takes time to collect data; so when we discuss that, we have some data handy to look at?

@dougwilson
Copy link
Contributor

Ok, it read to me on how do we do it a, b, or c? I think we need to first have the TC meeting so we can actually bring you to to speed on our past discussions on this topic so we can hear your and you hear ours and then, once there is a meeting of the minds, determine our path forward.

@gireeshpunathil
Copy link
Author

ok. I see there are 2 types of things:

  • items that needs action, no discussion / consensus required as such : (e.g:- path-to-regexp doc update) - can be worked out with community members, independently.

  • items that needs discussion and consensus (e.g:- release cadence) - TC meeting to cover in its agenda.

@gireeshpunathil
Copy link
Author

Just came up with a template for the release schedule, that we can have a look, brainstorm and bless in the next TC!

  • Feel free to add items that you believe needs to be met, by editing this comment.
  • If you are only proposing, probably fill only the first (and second?) columns
  • if you are both proposer and owner, fill all the columns
  • if you are owning the proposed work, fill the rest of the columns

WEEK 1

item name tracking issue owner challenge/support required eta

WEEK 2

item name tracking issue owner challenge/support required eta

WEEK 3

item name tracking issue owner challenge/support required eta

WEEK 4

item name tracking issue owner challenge/support required eta

@wesleytodd
Copy link
Member

Can you elaborate on what challenge/support required means?

@gireeshpunathil
Copy link
Author

@wesleytodd -

Can you elaborate on what challenge/support required means?

challenge: anything that the owner wants to callout as a potential blocker in realizing the item
support required: anything that the owner request from the community members in help fulfilling the item

@wesleytodd
Copy link
Member

Figured I would follow up here on the req/res stuff. It was on our short list which we I thought we all agreed to. The iteration for v6 is my biggest concern. If we think re importing at the existing files is not a breaking change I am good with that. I believe it should not break anything. The work for it is quick as I already have it half done. I can do it tonight if we are all in agreement. Are there any other concerns than just if it could be done in time?

@gireeshpunathil
Copy link
Author

If we think re importing at the existing files is not a breaking change I am good with that.

@wesleytodd - do you mean the approach @jonchurch suggested in #106 (comment) ?

@gireeshpunathil
Copy link
Author

re: #100 (comment)

given that it is a day's work, and it is not going to break anything, I am fine with that. thanks @wesleytodd !

@gireeshpunathil
Copy link
Author

track the progress of items planned for the week:

@gireeshpunathil
Copy link
Author

I thougth we are going to have a TC sitting (my) today (12 hours from now) , but do not see a tracking issue. @jonchurch - are you going to setup one?

@dougwilson
Copy link
Contributor

Yes @wesleytodd I think pillarjs org would make sense for them; since they are along the lines of "framework building blocks" and that is the goal of pillarjs, it makes the most sense to me. I could see it in expressjs as well, but so far that has been pretty much confined to express itself and middlewares. Along the lines of thinking of another framework using it, even for "Express compatibility" that also makes me think it should be in pillarjs.

It would be nice is there would be a place to discuss the actual architecture of those repos ideally. I think the biggest feedback I have from reviewing them is they... do not actually work independent of each other, yet they are published independently as if they could be. I'm not clear on if the issue is either they they depend on each other by accident or maybe that they should just be a single repo and not two separate repos.

The readme of express-request seems to imply it should be able to be used stand alone, but yet following the example and attempting to run it promptly fails due to there being no this.app defined. Trying to use req.fresh also fails, not just because this.res is not defined, but also because the res is expected to also have been wrapped with the separate package (which it itself has methods expecting req to be wrapped with the other package).

My point above is specifically not to discuss issues / changes with those repos here, rather to give an example about something that would be nice to discuss somewhere, and putting it out for suggestion on what would be a place to have said discussions.

@gireeshpunathil
Copy link
Author

3rd TC meeting happened on 18th #109 with agenda as outlined in #109 (comment)

agreed action items:

@gireeshpunathil
Copy link
Author

@jonchurch / @wesleytodd / @dougwilson

could one of you pls spin up a TC meeting for this week?

@gireeshpunathil
Copy link
Author

current status of ear-marked and priotized items:

pillarjs/router#42 : landed

expressjs/express#4208: ready to land since last 19 days
expressjs/express#4210: ready to land since last 20 days
expressjs/express#4212: under review

expressjs/expressjs.com#1114: ready to land since last 20 days
https://github.com/pillarjs/request: repo created. not sure if anything else is pending

expressjs/expressjs.com#1127: needs review, since 11 days
expressjs/expressjs.com#1128: merged
expressjs/expressjs.com#1129: needs review, since 4 days
expressjs/expressjs.com#1130: needs review, since 4 days
expressjs/expressjs.com#1131: needs review, since 10 days
expressjs/expressjs.com#1132: needs review, since 9 days
expressjs/expressjs.com#1134: needs review, since 4 days

can I request some focus on moving these please? (involves code reviews and merges) @expressjs/express-tc

@gireeshpunathil
Copy link
Author

Reduced list after today's TC meeting:

expressjs/express#3621: good to land
expressjs/express#4208 : no concerns from anyone on not printing the deprecation. Close and abandon.
expressjs/express#4212: needs review and progress
expressjs/expressjs.com#1114: post release action
expressjs/expressjs.com#1131: needs review and progress

@expressjs/express-tc - please chime in! we are getting closer, and this now needs few landings + the release process!

note 1: there isn't anything major between the last alpha and the current, so I strongly recommend for GA, not a beta.

note 2: the risk of delaying further is that new issues can slip in from unknown corners with potential of delaying and blocking things.

@gireeshpunathil
Copy link
Author

expressjs/expressjs.com#1131 is landed now, so we are down to 4!

@dougwilson
Copy link
Contributor

In order to jump from alpha to GA, we will need to finish up the feature in the router repo, get the body parser 2.0 out to match the query praser change in 5.0, and get all those deps to be non beta release as well, as a GA express shouldn't have non GA deps. I believe the idea of a single beta is so we can release one intermediate express 5 between those, but if we want to hold the next 5 release for those things in order for it to be a GA release, we can certainly do so.

@gireeshpunathil
Copy link
Author

@dougwilson -

In order to jump from alpha to GA, we will need to finish up the feature in the router repo, get the body parser 2.0 out to match the query praser change in 5.0, and get all those deps to be non beta release as well, as a GA express shouldn't have non GA deps.

this is surprising! where do this come from? I have proposed GA 2 months back here #100 (comment) and this was not raised!

GA express shouldn't have non GA deps

where is this documented?

we will need to finish up the feature in the router repo, get the body parser 2.0 out to match the query praser change in 5.0, and get all those deps to be non beta release as well, ...

are there issues / PRs opened for these? or need to start from scratch?

@dougwilson
Copy link
Contributor

Yes, we know there is a lot of gaps in our docs, and I know myself and Wes have been trying hard to continue to push forward with the work needed and still try and provide as much information as possible. We are trying out best, but we do missing things. If you think it is desirable, we can go ahead and stop pushing forward on the 5.0 work and pivot into perhaps a summit-style type of thing to get everything that needs to be done across all three orgs completely documented.

Let me know what you think is a best use of time. We are here trying to best move everything forward, and with our limited resources we have at the moment. We built up our group quickly, with a sudden focus to release 5.0 as fast as possible, and perhaps we are cutting corners we should not, like fully articulating the plan.

I think perhaps we need to stop the work towards 5.0 at this point and focus on a full plan of all the sub-dependency work across the three orgs that we are holding in our minds so we can get a full picture of what is being done and the plan.

Does this sound like a good proposal? Do you have an alternative proposal?

As for the request of non-GA deps in GA code, we don't document it, but perhaps you're saying we should. Typically this is just, I believe, an expectation for Node.js modules; that when you npm install express it doesn't pull in a bunch of alpha- and beta-marked dependencies along with it when you didn't ask for an alpha- or beta-marked express? I feel like that would be quite surprising to see (GA code depending on non-GA code), but perhaps I am misunderstanding what it means to be GA? Perhaps we should fully document and articulate what going GA means before we proceed in order to understand that we meet the GA definition with our GA release 🤔

@gireeshpunathil
Copy link
Author

@dougwilson -

We built up our group quickly, with a sudden focus to release 5.0 as fast as possible, and perhaps we are cutting corners we should not, like fully articulating the plan.

I wouldn't call it sudden - it has been 2 months since this issue is opened, to say the least. And there were several express 5.0 plans in the past as well?

I think perhaps we need to stop the work towards 5.0 at this point and focus on a full plan of all the sub-dependency work across the three orgs that we are holding in our minds so we can get a full picture of what is being done and the plan.

No. Lot of contributors spent a lot of effort to get this out. Let us don't think of any other actions that will delay it any further. I am understanding now that there are several gaps in the documentation, process and other nagging problems that needs collaborative effort to improve upon, but let us don't allow those to derail the current focus and larger goal.

we will need to finish up the feature in the router repo, get the body parser 2.0 out to match the query praser change in 5.0, and get all those deps to be non beta release as well, ...

are there issues / PRs opened for these? or need to start from scratch?

@dougwilson
Copy link
Contributor

Yes, those repos have them as open issues and PR (mainly PRs targeting the milestone).

@gireeshpunathil
Copy link
Author

pillarjs/router#60 : PR to make router 2.0 . All the checklists are marked, yet not sure what it is waiting for?

expressjs/body-parser#66: PR to release body-parser 2.0: how this is a pre-requisite for query praser change in 5.0?

@dougwilson
Copy link
Contributor

dougwilson commented Apr 23, 2020

Hi @gireeshpunathil is it OK for me to delay the responses to your questions until next week? I would like to focus on 4.18 tonight/tomorrow if that is OK.

@gireeshpunathil
Copy link
Author

I would like to resume this work. @dougwilson - when you get time, please let me know the latest list pre-requisites for 5!

@dougwilson
Copy link
Contributor

dougwilson commented Jul 8, 2020

Hi @gireeshpunathil the PR for the 5.0 release has a checklist tree that should be up-to-date with that information. If not, please bring up what is outdated there. All the items we dropped from there in previous conversations have a strike out to indicate that they were removed from consideration on purpose to shrink the scope.

Edit: I tried to add a note on the lines where a particular change is coming in from a dependency update, to help with it a bit since we have a lot of the core Express functionality broken out into different dependencies.

@gireeshpunathil
Copy link
Author

I have extracted the outstanding items from expressjs/express#2237 (comment) as:

with the last one opened today, seeing no other open PRs on it. Next I will review each of the existing PRs to see where do they stand, and what actions required to move those forward

@dougwilson
Copy link
Contributor

I am locking this issue until the communications issues between myself and @gireeshpunathil are resolved.

@expressjs expressjs locked as too heated and limited conversation to collaborators Jul 27, 2020
@UlisesGascon
Copy link
Member

Next phase is in #233

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discuss top priority Issues which the TC deem our current highest priorities for the project
Projects
None yet
Development

No branches or pull requests

5 participants