balena-io-modules / etcher-sdk

Apache License 2.0
42 stars 17 forks source link

add support for basic auth #212

Closed flec closed 3 years ago

flec commented 3 years ago

This adds support for basic auth required by https://github.com/balena-io/etcher/pull/3581. The doc has not yet been created since it would change all the URLs to the wrong repository. How could this be resolved?

The reason for this change instead of using the axiosInstance property is that I did not want to introduce axios as another direct dependency to etcher.

thundron commented 3 years ago

thank you very much for the PR, I'll make sure to evaluate it and check a few things internally so I can get back to it as soon as possible; it does look better than using the axios instance, I just have to make sure we're in line for a new feature that is going to be enabled with the authorization which is a "Flash with Etcher" button that we want to let people use to directly open an image from the internet with Etcher (and this sounds like it's in that direction definitely!) for the time being, just a minor change to the commit itself: you'll need to either prefix the title with patch: ... or include a Change-type: patch footer tag to the commit

flec commented 3 years ago

Thank you very much for your review. I've updated the commit message. Do you have any suggestions on how I can update the doc without changing any of the URLs or are you doing this after the merge?

thundron commented 3 years ago

cherry-picked in https://github.com/balena-io-modules/etcher-sdk/pull/213

as for your question: there is a npm command listed in the package.json that is used to rebuild docs, but it wasn't working as intended on all OSes; I made sure to fix it, I'm going to test it on the PR I linked above so you don't have to :+1: