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

Update httpsnippet, add Axios, fetch() and Kotlin OkHttp #540

Closed
philsturgeon opened this issue Aug 18, 2020 · 4 comments
Closed

Update httpsnippet, add Axios, fetch() and Kotlin OkHttp #540

philsturgeon opened this issue Aug 18, 2020 · 4 comments

Comments

@philsturgeon
Copy link
Contributor

philsturgeon commented Aug 18, 2020

Evaluate how used the existing Node JavaScript libraries are (I've never heard of them) and add Axios.

  {
    name: 'Node',
    mode: 'javascript',
    codechoice: 'node',
    libraries: [
      { name: 'Native', librarychoice: 'native' },
      { name: 'Request', librarychoice: 'request' },
      { name: 'Unirest', librarychoice: 'unirest' },
    ],
  },

Axios was added to httpsnippet in Kong/httpsnippet#176 which was released as part of v1.22.0.

I'd also suggest changing the Browser JavaScript targets to include fetch() added in Kong/httpsnippet#128.

  {
    name: 'JavaScript',
    mode: 'javascript',
    codechoice: 'javascript',
    libraries: [
      { name: 'Fetch', librarychoice: 'fetch' },
      { name: 'jQuery', librarychoice: 'jquery' },
      { name: 'XMLHttpRequest', librarychoice: 'xmlhttprequest' },
    ],
  },

Aaaand Kotlin - OkHttp?

Maybe if we've not looked into this already, we could look into getting off our fork, so we don't have more libraries to maintain and bump when we want to add things like this.

@philsturgeon philsturgeon changed the title Update httpsnippet, add Axios Update httpsnippet, add Axios, fetch() and Kotlin OkHttp Aug 18, 2020
@marcelltoth
Copy link
Contributor

Maybe if we've not looked into this already, we could look into getting off our fork, so we don't have more libraries to maintain and bump when we want to add things like this.

I evaluated this and I can agree with your proposal.

It looks like they started maintaining the upstream to some degree, as you pointed out there were some real improvements made pretty recently. AFAICT the only commit specific to our repo is now stoplightio/httpsnippet@629566c @lottamus maybe you know what this is, and whether we can live without.

I'd vote for dropping the fork if we can, it's just another repo and package to maintain with no important benefit probably.

@lottamus
Copy link
Contributor

maybe you know what this is, and whether we can live without.

@marcelltoth I believe they've addressed it with Kong/httpsnippet#173, so we should be good with dropping the fork now.

@philsturgeon
Copy link
Contributor Author

Thank you @marcelltoth and @lottamus! We go through occasional bursts of trying to reduce our plethora of forks and anything we can remove helps maintenance overhead. Let’s try and continue to change the approach at stoplight so we aim to fork as a last resort, and send Luke requests whenever we have improvements to make to other stuff. 👍

@marcelltoth
Copy link
Contributor

Closing in favor of #628

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

No branches or pull requests

3 participants