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

fix load_cookies local name error #2355

Merged
merged 1 commit into from Sep 16, 2017

Conversation

Projects
None yet
3 participants
@lilydjwg
Copy link
Contributor

lilydjwg commented Sep 13, 2017

I've also run pylint so there should not be more obvious errors like this one.

fixes #2351

@soimort-bot

This comment has been minimized.

Copy link
Collaborator

soimort-bot commented Sep 13, 2017

Hello @lilydjwg,
Thanks for the Pull Request. We ❤️ our contributors!
Please wait for one of our human maintainers to review your patches. This may take a few days to weeks. Also, please understand that although your Pull Request may or may not be eventually merged, we value all contributions equally.

祝您健康!

@lilydjwg

This comment has been minimized.

Copy link
Contributor

lilydjwg commented Sep 13, 2017

The Python 3.2 test has errored without reason.

@soimort

This comment has been minimized.

Copy link
Owner

soimort commented Sep 13, 2017

json is used implicitly by many extractors (which do not import json but import common instead):

  • yinyuetai.py
  • tudou.py
  • sina.py
  • imgur.py
  • twitter.py
  • dailymotion.py
  • instagram.py
  • douban.py

You cannot remove this from common.py.

@lilydjwg lilydjwg force-pushed the lilydjwg:fixes branch from 8eee8b8 to d1f98fa Sep 13, 2017

@lilydjwg

This comment has been minimized.

Copy link
Contributor

lilydjwg commented Sep 13, 2017

Oh....I've removed that commit.

BTW, import * is really a terrible idea that I will never do again.

@lilydjwg

This comment has been minimized.

Copy link
Contributor

lilydjwg commented Sep 13, 2017

Travis-CI doesn't send mails for failed pr?...

lilydjwg added a commit to lilydjwg/you-get that referenced this pull request Sep 14, 2017

Revert argparse, back to getopt
Because "moving from gnu_getopt to argparse brings more problems than it
solves, if it really solves any". It contains "grammar errors brought with
by a new patch", doesn't fix "something defected exsited in the old
code". "there is no reason to blame the old code."

What's more, "argparse is much more complex with a lengthy document", it
makes you-get pythonic, but we need to make "Those who only know js or
as3 can contribute because you-get is not pythonic".

"This patch is premature and personal taste."

If you read the help again and again to find a flag that you know
you-get has but failed, you can just read the source code. And if you're
familiar with "js or as3", probably you can contribute to fix it.
"keeping description up-to-date" isn't a tough task after all.

This reverts commit bcd8d74.

This commit closes soimort#2355 and probably soimort#2145 too.

lilydjwg added a commit to lilydjwg/you-get that referenced this pull request Sep 14, 2017

Revert argparse, back to getopt
Because "moving from gnu_getopt to argparse brings more problems than it
solves, if it really solves any". It contains "grammar errors brought with
by a new patch", doesn't fix "something defected exsited in the old
code". "there is no reason to blame the old code."

What's more, "argparse is much more complex with a lengthy document", it
makes you-get pythonic, but we need to make "Those who only know js or
as3 can contribute because you-get is not pythonic".

"This patch is premature and personal taste."

If you read the help again and again to find a flag that you know
you-get has but failed, you can just read the source code. And if you're
familiar with "js or as3", probably you can contribute to fix it.
"keeping description up-to-date" isn't a tough task after all.

This reverts commit bcd8d74.

This commit closes #2351, soimort#2355 and probably soimort#2145 too.

@soimort soimort merged commit d1f98fa into soimort:develop Sep 16, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment