Closed isteves closed 6 years ago
This looks good to me! Thanks
Also, now that we know about demo.bestpractical.com, do you think it's worth writing tests based on that?
Maybe not. I wouldn't want to direct any extra traffic to their RT instance (via testthat or Travis CI) unless we asked first.
The output for format = "l" isn't super neat (it needs cat() to view properly), so if you have brighter ideas, let me know!
I see that. I think this is a great use case for S3. We could S3-ize much of this package to provide specialized print methods. It's kind of a tidyverse thing to never print too much stuff out to the user unless they de-tidyverse it which I generally like and only sometimes hate. Example would be tibbles which automatically only show you a few rows.
For the full history, I'm scratching my head thinking about what the most useful format would be. cat()
alone helps clean things up pretty well. Something like this is a good start:
> rt_function <- function() {
+ response <- list(status = 200,
+ content = "a\nb\nc\n")
+ structure(response, class = "rt_response")
+ }
> print.rt_response <- function(response) {
+ cat(response$content)
+ }
> some_response <- rt_function()
> some_response
a
b
c
What do you think about that kind of approach?
Also: Happy to merge this right away. Just give me a 👍 and I will.
Hmm interesting. Perhaps we can skip it for now, and maybe implement it at a later stage. I still don't fully understand the OO stuff, but I'll think about how we might want to implement something like this as I work through the other examples. If it's good to you otherwise, please merge! :shipit:
Sounds good!
Updated
rt_history
to fit new package style. @amoebaThe output for format = "l" isn't super neat (it needs
cat()
to view properly), so if you have brighter ideas, let me know!Also, now that we know about demo.bestpractical.com, do you think it's worth writing tests based on that? I'll add our github user info in as a next step.