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

feat: add setupRequest per request for requests array #239

Merged
merged 2 commits into from Jan 25, 2020
Merged

feat: add setupRequest per request for requests array #239

merged 2 commits into from Jan 25, 2020

Conversation

alex-ppg
Copy link
Contributor

Hey all,

Since the setupClient appears to mutate the Client classs located in httpClient.js, I decided to add the ability to mutate each request independently within the requests array if it is provided.

I thought about using the setupClient property at first, but it appears that each request in the requests array is passed to the RequestIterator and is never constructed as a Client presumably for performance reasons. For this purpose, I decided to name a new property called setupRequest that mutates the raw request object.

I have added corresponding tests, documentation and have followed the code style in autocannon. I hope this is a meaningful addition, as I myself did require this functionality in one of my projects where I tested the creation of entities and their subsequent mutation via a url param.

Please let me know if I should change anything in the PR to match your expectations.

Copy link
Owner

@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.

would you mind to remove package-lock and add it to .gitignore? Thanks!

@mcollina
Copy link
Owner

Good work on the feature, the code seems ok for me.

@alex-ppg
Copy link
Contributor Author

alex-ppg commented Jan 24, 2020

Done, let me know if anything else is needed.

EDIT: Renaming the commit to conform to the style of the other commits in a sec - Done

Copy link
Owner

@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 merged commit 7a8c4f9 into mcollina:master Jan 25, 2020
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.

None yet

2 participants