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.

Sign up for GitHub

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

Jump to bottom

Add facebook plugin #101

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add facebook plugin #101

wants to merge 7 commits into from

Conversation

mbajur
Copy link
Contributor

@mbajur mbajur commented Jun 4, 2014

No description provided.

@rb2k
Copy link
Owner

rb2k commented Jun 5, 2014

Thanks!
It seems like test_facebook() seems to fail in that travis run. Did it pass locally for you?
Maybe it's because of different headers during the download attempt OR you'd have to set the :method option to GET instead of the default HEAD.

@mbajur
Copy link
Contributor Author

mbajur commented Jun 5, 2014

Ok @rb2k , it was caused by the fact that facebook video url needs to be put between the brackets (when using viddl-rb shell command, ampersands are not parsed well without the brackets). I've also added SSL urls support for specs cause the ones coming from facebook have https:// prefix.

@rb2k
Copy link
Owner

rb2k commented Jun 5, 2014

Since your other PR was accepted, it seems like you'll have to rebase this on master so it can be merged

@mbajur
Copy link
Contributor Author

mbajur commented Jun 5, 2014

Well, that was actually first rebase in my life so i'm not sure if i've done everything right. Hope so.
Sorry for the troubles!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mbajur @rb2k