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

[WIP] Adds nodejs-axios codegen #245

Closed
wants to merge 6 commits into from

Conversation

someshkoli
Copy link
Contributor

@someshkoli someshkoli commented May 5, 2020

Closes #135
Adds support for axios codegen as requested in #135
snippet samples:
post req:
image
get req:
image

 - added axios support
@someshkoli
Copy link
Contributor Author

hey @shreys7 , the build is failing due to timeout in java-okhttp can you check once if there's something missing ?

@emanuelet
Copy link

possible duplicate, #232

@someshkoli
Copy link
Contributor Author

someshkoli commented May 6, 2020

Yeah, we both made pr almost at the same time without knowing each other's work. There's a closed PR #236. Had to close this as the branch was bit messed up.

// Compile script required to compile the code snippet
compileScript: '',
// Array of name of collections for which newman tests has to be skipped.
skipCollections: ['sameNameHeadersCollection', 'formdataCollection', 'formdataFileCollection']
Copy link
Member

Choose a reason for hiding this comment

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

Why are these collections skipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't actually remember :. ill test again maybe

@shreys7
Copy link
Member

shreys7 commented May 7, 2020

I think URL, method, body, etc. can also be provided in the config object of Axios. Can you check it once? If possible, can you make the required changes.

@someshkoli
Copy link
Contributor Author

Yeah it can be done. Axios can be used in 2 ways and this is one of them and ig majority of people use this approach.

@shreys7
Copy link
Member

shreys7 commented May 8, 2020

We will look at it and take it up if the users are requesting the other approach. For now, we would like to have method, URL, body, etc. to be part of the config object @someshkoli

@shreys7
Copy link
Member

shreys7 commented May 11, 2020

any updates @someshkoli ?

@someshkoli
Copy link
Contributor Author

Nah the tests were still failing. Was waiting for your pr to get merged #246.

@shreys7
Copy link
Member

shreys7 commented May 12, 2020

I am asking about the configuration part. The body, method etc.

@someshkoli
Copy link
Contributor Author

Yeah it is

@someshkoli
Copy link
Contributor Author

@shreys7 I'm stuck at one request. And since there is already a perfect implementation ready #232 . Can you please merge his version.

@shreys7
Copy link
Member

shreys7 commented May 13, 2020

Okay. As you have good enough context too, can you go through #232 and add your sanity review there? @someshkoli

@someshkoli
Copy link
Contributor Author

Sure I'll do that.

@shreys7
Copy link
Member

shreys7 commented May 20, 2020

Closing this, focussing on #232

@shreys7 shreys7 closed this May 20, 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.

Can you please add the support for axios
3 participants