Skip to content

Integrate cookie, use improved cookie parsing method#152

Merged
Eomm merged 4 commits intofastify:masterfrom
Uzlopak:integrate-cookie
Nov 23, 2021
Merged

Integrate cookie, use improved cookie parsing method#152
Eomm merged 4 commits intofastify:masterfrom
Uzlopak:integrate-cookie

Conversation

@Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Nov 22, 2021

As we discussed in #151 here is a PR for the change.

How should we proceed with the documentation? Should I copy paste it into the Readme.md or is the reference to the original repo OK?

Are the credits just in the cookie.js OK? Or is there a guideline on how to credit?

Checklist

integrate cookie package
integrate cookie tests
rename cookie.test.js to plugin.test.js
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Good work

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested a review from Eomm November 23, 2021 16:38
@Eomm Eomm merged commit 4ef74d8 into fastify:master Nov 23, 2021
@Uzlopak Uzlopak deleted the integrate-cookie branch November 23, 2021 18:24
kurtextrem added a commit to kurtextrem/fastify-cookie that referenced this pull request Jul 14, 2022
Previously, in PR fastify#152 we removed the `cookie` dep., since the repo was stale.
The cookie module seems to be active again, and now actually the current implementation was outdated and lacking a performance fix from ~4 months ago.
@kurtextrem kurtextrem mentioned this pull request Jul 14, 2022
4 tasks
mcollina pushed a commit that referenced this pull request Jul 14, 2022
Previously, in PR #152 we removed the `cookie` dep., since the repo was stale.
The cookie module seems to be active again, and now actually the current implementation was outdated and lacking a performance fix from ~4 months ago.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants