Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Vimeo v3 #87

Merged
merged 6 commits into from
Aug 5, 2015
Merged

Feature Vimeo v3 #87

merged 6 commits into from
Aug 5, 2015

Conversation

zdb
Copy link
Contributor

@zdb zdb commented Jul 17, 2015

Updates Vimeo provider to version 3 of Vimeo's API.

Vimeo API v2 has been deprecated. Version 3 now requires an access token for API calls. This also allows video_info to fetch a users private videos.

@@ -122,22 +125,22 @@

describe '#date' do
subject { super().date }
it { is_expected.to eq Time.parse('2008-04-14 13:10:39', Time.now.utc) }
it { is_expected.to eq Time.parse('2008-04-14T17:10:39+00:00', Time.now.utc).utc }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [88/80]

@thibaudgg
Copy link
Owner

@zdb thanks for the PR, I'll have a look once PR #79 has been merged (that'll fix travis failures)

@thibaudgg
Copy link
Owner

Master should be back to green now, could you rebase please? Thanks!

@vheuken
Copy link
Collaborator

vheuken commented Aug 4, 2015

@zdb I rebased your changes with master. I pushed it to branch feature-vimeo-v3 on @thibaudgg's repo.

It looks like some things are failing:

  1) VideoInfo::Providers::VimeoPlaylist with playlist 1921098 #thumbnail_small should eq "https://i.vimeocdn.com/video/1921098_100x75.jpg"
     Failure/Error: it { is_expected.to eq 'https://i.vimeocdn.com/video/1921098_100x75.jpg' }

       expected: "https://i.vimeocdn.com/video/1921098_100x75.jpg"
            got: "https://i.vimeocdn.com/video/299773432_100x75.jpg"

       (compared using ==)
     # ./spec/lib/video_info/providers/vimeo_playlist_spec.rb:125:in `block (4 levels) in <top (required)>'

  2) VideoInfo::Providers::VimeoPlaylist with playlist 1921098 #thumbnail_medium should eq "https://i.vimeocdn.com/video/1921098_200x150.jpg"
     Failure/Error: it { is_expected.to eq 'https://i.vimeocdn.com/video/1921098_200x150.jpg' }

       expected: "https://i.vimeocdn.com/video/1921098_200x150.jpg"
            got: "https://i.vimeocdn.com/video/299773432_200x150.jpg"

       (compared using ==)
     # ./spec/lib/video_info/providers/vimeo_playlist_spec.rb:130:in `block (4 levels) in <top (required)>'

  3) VideoInfo::Providers::VimeoPlaylist with playlist 1921098 #thumbnail_large should eq "https://i.vimeocdn.com/video/1921098_640.jpg"
     Failure/Error: it { is_expected.to eq 'https://i.vimeocdn.com/video/1921098_640.jpg' }

       expected: "https://i.vimeocdn.com/video/1921098_640.jpg"
            got: "https://i.vimeocdn.com/video/299773432_640.jpg"

       (compared using ==)
     # ./spec/lib/video_info/providers/vimeo_playlist_spec.rb:135:in `block (4 levels) in <top (required)>'

There are also a couple Hound-CI warnings outside of the specs that don't seem to have been resolved.

Can you have a look at these? Thanks!

@vheuken
Copy link
Collaborator

vheuken commented Aug 4, 2015

Also, it looks like this doesn't work without an API key, right? If so, should we make an issue for a Vimeo scraper, @thibaudgg?

@thibaudgg
Copy link
Owner

@vheuken a scraper sounds good!

@zdb
Copy link
Contributor Author

zdb commented Aug 4, 2015

@vheuken Thanks for that. Yes, an API key is required for the v3 API. I'll look into the test that are failing.

@vheuken vheuken mentioned this pull request Aug 5, 2015
zdb added 5 commits August 5, 2015 13:47
Vimeo API v2 has been deprecated.  Version 3 now requires an access token for API calls.  This also allows video_info to fetch a users private videos.

Access tokens can be obtained here: https://developer.vimeo.com/apps
end

describe '#thumbnail_medium' do
subject { super().thumbnail_medium }
it { is_expected.to eq 'http://i.vimeocdn.com/video/299773432_200x150.jpg' }
it { is_expected.to eq 'https://i.vimeocdn.com/video/299773432_200x150.jpg' }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [83/80]


def thumbnail(width = 200, height = nil)
base_uri = "https://i.vimeocdn.com/video/#{thumbnail_id}"
height ? base_uri + "_#{width}x#{height}.jpg" : base_uri + "_#{width}.jpg"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [82/80]

@vheuken
Copy link
Collaborator

vheuken commented Aug 5, 2015

I was going to comment on the HoundCI warnings, but those seem to all be from strings that are slightly too long, which isn't a big deal.

Thanks!

vheuken pushed a commit that referenced this pull request Aug 5, 2015
@vheuken vheuken merged commit a8deec1 into thibaudgg:master Aug 5, 2015
@zdb
Copy link
Contributor Author

zdb commented Aug 5, 2015

Thanks for the merge.

@thibaudgg
Copy link
Owner

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants