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

Bugfix of -f option. #50

Closed
wants to merge 9 commits into from
Closed

Bugfix of -f option. #50

wants to merge 9 commits into from

Conversation

SaitoAtsushi
Copy link

This branch contains bugfix of -f option.
Regardless of the specification format value, previous youtube-dl download same video.
Cause of the problem is watching the conn.
I changed from conn to fmt_stream_map.

@rg3
Copy link
Collaborator

rg3 commented Jan 3, 2011

There is no way I'm pulling this, for many reasons.

There should be only one commit to pull, and a fast-forward merge is preferred. Rebase if needed.

The "Ignore download errors" commit is completely bogus. There's a command line option to ignore download errors.

I don't fully understand what you say in the pull request description, and what you mention cannot be easily found in the commits. I noticed a change from fmt_url_map to fmt_stream_map, but not conn. This is competely critical for the code to be merged.

You actually commented the Accept-Encoding header, reverting the changes from my own commit without any notice whatsoever!

You commited a change, then reverted it in a different commit (the "debug print" stuff).

Some commit messages leave a lot to be desired, like "bug fix" and "erase debug print" (lack of capitalization and what they do, in general).

You included a lot of stuff about the YouTube user InfoExtractor, which is nice but belongs to a completely different thing and should be handled in a separate pull request. I'm not discouraging you from submitting these changes, but they do not belong here.

@SaitoAtsushi
Copy link
Author

Thanks for review.
My English skill is not enogh.
I could not describe commit message adequately.
It's same for git and python.
I am referring to your pointed out, I will start over.

This pull request was closed.
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

3 participants