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

[utils] Only invite error reports from competent users #30142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dirkf
Copy link
Contributor

@dirkf dirkf commented Oct 22, 2021

Please follow the guide below


Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

Various other applications and services embed youtube-dl and its forks, in way that may present error messages designed for direct users of youtube-dl to the users of such embedding applications and services. Since such users ran the embedding application or connected to the embedding service, they are generally not competent to respond to an error message that asks them to run youtube-dl with certain command-line options and therefore cannot be helped to solve the issue. They should instead turn to the support channel for the embedding application and service.

Examples: #30140, https://github.com/ytdl-org/youtube-dl/issues?q=is%3Aissue+telegram.

This PR rewords the default postfix message for errors to direct consumers of embedding applications and services away from the youtube-dl bug tracker and towards the support channel for the embedding application and service.

@Ashish0804
Copy link

implying people will not just ignore it

🤦‍♂️

@fstirlitz
Copy link
Contributor

Indeed; that message is long enough as it is, my eyes already glaze over most of it. Better would be to modify the message to point to the issue tracker only in the CLI frontend (__main__), and then only if os.isatty(1) and os.isatty(2). And that’s assuming this pull request will be merged in the first place, which looks less likely by the day.

Also, while the error message itself is otherwise okay, the wording of the pull request title/commit description is rather unfortunate. You’re basically calling users idiots.

@dirkf
Copy link
Contributor Author

dirkf commented Oct 23, 2021

While that might sometimes be fair, 'competent' should be interpreted in the legal sense as 'having the jurisdiction, capacity and resources'. The embedded context typically denies that to the user.

point to the issue tracker only in the CLI frontend (main), ...

The embedder may be running the yt-dl program rather than a Python program that imports it.

and then only if os.isatty(1) and os.isatty(2)

Well, that may be a better idea, but wouldn't we like to tell the failing app about the error?

@fstirlitz
Copy link
Contributor

Then tell it. Just don’t include the bug report instructions.

And not from consumers of applications or services that embed yt-dl, whose owners should carry the can
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.

3 participants