Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

perf: replace querystring with fast-querystring #65

Merged
merged 7 commits into from
Oct 27, 2022

Conversation

anonrig
Copy link
Contributor

@anonrig anonrig commented Sep 9, 2022

Checklist

@anonrig
Copy link
Contributor Author

anonrig commented Sep 9, 2022

@mcollina

@Eomm
Copy link
Member

Eomm commented Sep 11, 2022

following the other prs fastify/fastify-reply-from#272

Could you please add an option to configure the querystring module to use? Like in Fastify.

@anonrig
Copy link
Contributor Author

anonrig commented Sep 12, 2022

@Eomm There's already an option to support this: https://github.com/fastify/fast-proxy#querystring

@anonrig
Copy link
Contributor Author

anonrig commented Sep 13, 2022

I released 1.0 and updated the pull request.

@Fdawgs Fdawgs requested a review from Eomm September 15, 2022 10:50
@anonrig
Copy link
Contributor Author

anonrig commented Sep 20, 2022

Any updates on this pull-request? @Eomm

@Uzlopak
Copy link
Contributor

Uzlopak commented Sep 20, 2022

Actually the remark by eomm is not solved. queryString option is an option to replace the querystring of a request it seems. But it is not an option to replace the querystring "engine" itself.

@anonrig
Copy link
Contributor Author

anonrig commented Sep 20, 2022

Yes, I totally missed that. I'll update the pull request. Thanks for the correction @Uzlopak

@anonrig
Copy link
Contributor Author

anonrig commented Sep 22, 2022

I added queryString parameter and updated both tests and docs. Can someone approve the workflow to run?

index.js Outdated Show resolved Hide resolved
test/4.opts.test.js Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@anonrig anonrig requested review from Eomm and RafaelGSS and removed request for Eomm and RafaelGSS October 25, 2022 18:32
@anonrig anonrig requested review from RafaelGSS and Eomm and removed request for RafaelGSS October 25, 2022 18:32
@anonrig
Copy link
Contributor Author

anonrig commented Oct 25, 2022

I think it's time to revive this pull request. Something is wrong with Github and requesting reviews. @RafaelGSS @Eomm Can you review it again please?

@anonrig
Copy link
Contributor Author

anonrig commented Oct 25, 2022

I'd appreciate it if someone can approve the workflow, once again. Thank you.

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM.

@Eomm Eomm merged commit da80d9d into fastify:master Oct 27, 2022
@Eomm
Copy link
Member

Eomm commented Oct 27, 2022

@mcollina I cannot release this package

image

we should bump a major since there are 3 major versions that has been merged by automerge

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants