algorithmiaio / algorithmia-r

R Client for Algorithmia Algorithms and Data API
https://algorithmia.com/developers/clients/r/
MIT License
14 stars 10 forks source link

[INSIGHTS-13] Add Algorithmia Insights ability #5

Closed dinoboy197 closed 3 years ago

dinoboy197 commented 3 years ago

This has been tested by:

dinoboy197 commented 3 years ago

Moving back to draft as I am add some CI checks and getting things ready for CRAN submission.

dinoboy197 commented 3 years ago

One pre-existing test (loaderThrowsException) is failing now that CI is enabled for tests. Pinged @zeryx for assistance.

dinoboy197 commented 3 years ago

@algorithmiaio/developer-technologies-contractors ping for review - this is finally ready for you to review and approve! :smile:

zeryx commented 3 years ago

Is there no way to test/verify functionality locally @dinoboy197 ? I feel uneasy about oking a build where we can't verify locally, and we had to comment out a number of tests to get things to compile.

dinoboy197 commented 3 years ago

Is there no way to test/verify functionality locally @dinoboy197 ?

I think I'd have to ask you for one test:

The other two tests were written by James A who is no longer with us, and never went through PR review, so I'm not sure how he tested those:

It seems they were coded to talk directly to the public marketplace, which isn't a good practice, but unfortunately, that's in the past now.

I feel uneasy about oking a build where we can't verify locally

Do you mean to say that you don't think that CI running the tests in a clean environment is sufficient to prove that the existing tests work?

we had to comment out a number of tests to get things to compile

That's not quite accurate. We commented three tests because they did not pass, not because the runtime or test time code didn't compile.