search

danspratling / gatsby-source-multi-api

Allows data sourced from one or more rest api to be imported into a GatsbyJS project
12 stars 8 forks source link

Feat headers #20

Open mirzhasan opened 4 years ago

mirzhasan commented 4 years ago
mirzhasan commented 4 years ago

@danspratling please review (Can't assign you for some reason)

danspratling commented 4 years ago

Looks great! Thanks for adding this.

I've noticed the formatting doesn't match that in prettierrc. If you could run format that'd be great, but this is approved. Thanks for the update!

mirzhasan commented 4 years ago

@danspratling for being so quick. A couple of pointers/questions:

  1. Cool, yes i will format the code, and yes i'll update the readme to include this as well
  2. Do i need to run the build and merge that in as well?
  3. Am i publishing this or are you? If i am, is there any preference that you have?

On a side note, the right way to accomplish this would be to create a new breaking change PR that maches the API exactly like fetch, where method etc is inside the option. I didn't do it since it was a breaking change, but if we can align, i can do that as well.

danspratling commented 4 years ago

If you feel like that approach is correct and are happy to make the changes, I'm happy for you to do so.

I'll publish to npm after everything is complete. I really should automate it 😅

mirzhasan commented 4 years ago

@danspratling The requested changes are done. Feel free to merge it (I haven't change the package.json version)