search

kevin-m-kent / youtubeR

https://kevin-m-kent.github.io/youtubeR/
Other
2 stars 1 forks source link

Added function: get_videos_for_playlistId() #20

Closed jimrothstein closed 6 months ago

jimrothstein commented 9 months ago

Appreciate any feedback.

It does what I want it to but does need:

Thx.

kevin-m-kent commented 8 months ago

thanks for your PR and patience, I will look at these today. Sorry for the delay

kevin-m-kent commented 8 months ago

Thanks again for this one as well. I like the core functionality - I tested it locally and was able to succesfully get the video info. I also like the tibble function but I'm not sure where that fits in. Maybe it could be here but the name could be more descriptive like make_video_tibble.

I'm thinking that we would want to have that kind of tibble functionality for the output of many of the youtube responses....tells me we need some kind of an approach with generics. @jonthegeek any ideas?

kevin-m-kent commented 8 months ago

ah it looks like you added those functions here @jimrothstein . I will take a look

kevin-m-kent commented 8 months ago

I like what you have done. A couple of suggestions:

In terms of what each of these functions return, I am leaning towards not placing any filtering on the return object (like not filtering snippet fields). That way it has a lot of flexibility and can be used in multiple ways. I think this is also consistent with how @jonthegeek is approaching beekeeper.

But for now, I think it's fine if you just take a look at my two points above. Then once we merge, I can refactor a bit later so that everything is consistent.

jimrothstein commented 8 months ago

Fixed formattng ?get_playlists.