Fix: update README.md to align with default config.yaml for oauth#7
Conversation
adds oauth_client_id: '' and oauth_client_secret: ''
This reverts commit cb4d922.
mgoltzsche
left a comment
mgoltzsche
left a comment
There was a problem hiding this comment.
@varia-fur thanks for creating the PR!
Looks good but the documentation still needs another update.
| p.add_option('--cookiefile', type='string', metavar='FILE', | ||
| default=self.config['cookiefile'].get(), | ||
| dest='cookiefile', help='cookiefile to use') |
There was a problem hiding this comment.
The CLI documentation within the readme should be updated (dumping the help output of the CLI into the corresponding section within the README).
Did you test the new cookiefile option manually and verified that it makes the plugin really download the high quality audio?
There was a problem hiding this comment.
it did download a format that requires premium, yes
| oauth_client_id: '' | ||
| oauth_client_secret: '' |
There was a problem hiding this comment.
Please also add/document the cookiefile config option here.
Also, I think it would be good to keep all the authentication options listed together (next to auth_headers).
Also, would you mind briefly explaining in the README how to obtain the cookies file or link to another page that's explaining that?
(Similarly, we/I should document how to obtain the oauth client credentials but that's not blocking your PR.)
|
Also, the CI build failed due to the convention commits format not being followed. Ideally, you should fix that by rewriting/squashing your commits but in doubt alternatively I can do that when merging the PR. |
varia-fur
changed the title
|
have tried to squash the commits, tho don't know if i did so correctly |
2 participants
oauth_client_idandoauth_client_secretwithin the readme.--cookiefileoption.