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

Updated README.md #9946

Closed
wants to merge 17 commits into from
Closed

Updated README.md #9946

wants to merge 17 commits into from

Conversation

KPCOFGS
Copy link

@KPCOFGS KPCOFGS commented May 17, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

Updated relative links to absolute links for PyPI documentation compatibilities

ADD DESCRIPTION HERE

Fixes #

Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

  • 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?

@seproDev seproDev added the docs/meta/cleanup related to docs, code cleanup, templates, devscripts etc label May 17, 2024
@bashonly
Copy link
Member

This won't work, since multiple versions of the README exist on Github (e.g. master vs 2024.04.09 vs 8e15177b4113c355989881e4e030f695a9b59c3a etc), but all links would point to the master version.

The proper approach would be to process the README via script before publishing to PyPI. See #6271 for more discussion

@bashonly bashonly added the pending-fixes PR has had changes requested label May 17, 2024
@KPCOFGS KPCOFGS marked this pull request as draft May 17, 2024 18:37
@KPCOFGS KPCOFGS marked this pull request as ready for review May 17, 2024 18:46
@KPCOFGS
Copy link
Author

KPCOFGS commented May 17, 2024

Hello! Thank you for your feedback! I see what the problem is. One way that can potentially fix this problem is to change the part of the url where it says /master/ to a certain variable and then make a workflow that can automatically change that variable to the proper branch name.

I just created a GitHub workflow and tested it on my fork and apparently it's working as expected. On my forked README.md file, I changed my previously modified urls from master to VARIABLE, then the workflow changed VARIABLE back to the branch name

One downside is it may be too tedious, devs now needs to spend extra time changing urls to VARIABLE

@KPCOFGS KPCOFGS marked this pull request as draft May 17, 2024 18:57
@KPCOFGS KPCOFGS marked this pull request as ready for review May 17, 2024 19:00
@KPCOFGS
Copy link
Author

KPCOFGS commented May 17, 2024

Edit: Changed VARIABLE to THIS_VARIABLE in the workflow as the previous one resulted in conflicts

@pukkandan
Copy link
Member

Only the PyPI description should be changed - not the readme that is committed into the repo

@KPCOFGS KPCOFGS closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs/meta/cleanup related to docs, code cleanup, templates, devscripts etc pending-fixes PR has had changes requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants