fastify / session

Session plugin for fastify
Other
105 stars 49 forks source link

@fastify/session v10.0.0 or What is missing for the next version? #146

Closed Uzlopak closed 2 years ago

Uzlopak commented 2 years ago

Prerequisites

Issue

Alot of work has been invested into this plugin. But are we now feature complete? I have the feeling that we are so close to have it the biggest issues fixed.

On one hand I am kind of exhausted by the work on this plugin. So basically I read the unit tests, and dont understand them anymore. And on the other hand, I really want that we get a release soon.

So can we please consolidate the open issues in this thread? Or open new issues.

It would be good if you would read critically the code too. Maybe you see something we did overlook? Or is maybe totally wrong in the first place?

So give please feedback.

So I start, and if I find more cases or if you find some then please post them. I would add them to this list.

[ ] check if rolling is working as expected

Uzlopak commented 2 years ago

pinging people, who are maybe willing to give feedback: @rclmenezes @SimenB @cameronmbell @sarneeh @m0dch3n

rclmenezes commented 2 years ago

On one hand I am kind of exhausted by the work on this plugin. So basically I read the unit tests, and dont understand them anymore. And on the other hand, I really want that we get a release soon.

Totally agree! Honestly, I've been following this repo and it's been much cleaner to work with: https://github.com/mgcrea/fastify-session

I also experimented with importing Store and Session from express-session and plugging it in via fastify hooks. That way, we can make a separate repo for users who want express compatibility and we can stop worrying about replicating express-session's behavior here.

Anyway, I'm off-topic. The other things I think we should do:

rclmenezes commented 2 years ago

Also -- thank you @uzlopak for all your work so far!

SimenB commented 2 years ago

134 and #143 are the main breaking things I was waiting on, very happy to see them land 😀 Beyond that I don't really know what's missing (@rclmenezes's list looks good). As mentioned in https://github.com/fastify/session/pull/101#issuecomment-1191314027 we had issues with the data persisted in the session, but I still haven't found the time to dig into it. And regardless, I doubt the fix will be semver major (it might even have gotten fixed along the way if we're lucky 🙂)

climba03003 commented 2 years ago

I would like to know how many task are left? If there are no pending task or new comment, I will release a new major on this Friday 9 / 9.

Then, I will cut a next branch to prevent staling lots of breaking changes in master.

SimenB commented 2 years ago

Probably just missing https://github.com/fastify/session/pull/154?

Uzlopak commented 2 years ago

It seems Feature complete. But i would prefer not to release now. I want to just do final check. So i think friday is a good finish Line

Eomm commented 2 years ago

Are we ready to land?

Uzlopak commented 2 years ago

Please wait. I have found three small issues. PR are incoming

Uzlopak commented 2 years ago

I provided all three PRs.

168

169

170

After that I will not touch this plugin for a while :D

mcollina commented 2 years ago

@Uzlopak could you sum up in a paragraph all the changes you did for this release? A long list of PRs might be hard to digest.

Uzlopak commented 2 years ago

Based on the auto generated release notes I modified them. I thought you maybe want to copy paste that.


Relevant Changes

What also Changed

New Contributors

Full Changelog: https://github.com/fastify/session/compare/v9.0.0...v10.0.0

climba03003 commented 2 years ago

Released 16ef7eb6c79ecc2342b0611c3aebb2512ca0c24c

climba03003 commented 2 years ago

I have added a Notable Changes section on the release notes.

@Uzlopak Can you helps me to check if anything missing.