easystats / insight

:crystal_ball: Easy access to model information for various model objects
https://easystats.github.io/insight/
GNU General Public License v3.0
380 stars 38 forks source link

Implement `download_model()` using `{httr2}` and add tests #885

Closed IndrajeetPatil closed 3 weeks ago

strengejacke commented 3 weeks ago

Do we need both httptest2 and httr2 to use download_model(), or is httr2 sufficient?

IndrajeetPatil commented 3 weeks ago

httptest2 is for tests, httr2 provides the client.

The user needs only the latter.

strengejacke commented 3 weeks ago

thanks! we need to change this (httr -> httr2) in our tests where we use download_model(), that's why I'm asking which packages should go into skip_if_not_installed().

IndrajeetPatil commented 3 weeks ago

You are right!

Sorry, missed it in this PR. I should have just removed httr from DESCRIPTION.

strengejacke commented 3 weeks ago

You have removed it. I added it back for the patch-release. Else, this insight release would again break downstream packages 😬 and I thought it would be good not to cause any more despair. ;-)

So once all tests in other packages are fixed, we can remove httr (and the helper code I added back temporarily).

IndrajeetPatil commented 3 weeks ago

SGTM.