Closed sckott closed 4 years ago
Heya @sckott. I shoulda guessed this was going to happen.
I think it's totally reasonable to put it in Suggests since I don't think it's core functionality of this package. If memory serves, I was also thinking we might be able to remove the dependency altogether with a rewrite of the Spaces implementation but Suggests
is sounds good for now.
I'd be happy to PR this. Do we want to add warning
s to Spaces functions using the typical !requireNamespace
pattern too?
Yeah, lets submit a new version soon with move to suggests. PR sounds great. What do you mean by add a warning? we already have this fxn https://github.com/sckott/analogsea/blob/c517c4d4d960f78e43211c66bc17d7e15b41a2ce/R/zzz.R#L65-L71 if its appropriate
What do you mean by add a warning? we already have this fxn
Sorry, could've explained more. Something such that, if the user doesn't have aws.s3
installed, running code like...
spaces()
...emits a warning using check_for_a_pkg()
. I'm not sure I'd wanna plaster it across all exported Spaces functions. Maybe that's overkill?
I think it'd be better to stop with message rather than warn if the user doesn't have aws.s3 installed, yes?
Ah, yep. That makes a lot more sense!
^ PR'd.
since its been orphaned we have to put it in suggests (and only use conditionally) or not use it. thoughts @amoeba ?