Skip to content

Conversation

Behoston
Copy link

@Behoston Behoston commented Aug 11, 2021

Just a readme fix.

WARNING: python-telegram-bot 13.7 does not provide the extra 'ujson'

https://github.com/python-telegram-bot/python-telegram-bot/blob/master/setup.py#L85

@harshil21
Copy link
Member

Thanks for the spot, I think it makes more sense to change it to ujson in the setup.py

@Behoston
Copy link
Author

agree, [ujson] is much clearer, but you need to extend existing setup to support both to backward compatibility. I'm not sure if extend will be ok, so I just fix readme to match code.

@Bibo-Joshi
Copy link
Member

that's good thinking @Behoston ! however, we're by now working towards v14, which will break a lot of things anyway. So that wouldn't necessarily be a no-go. Anyway, while I somewhat like the idea to rename json to ujson, the former makes it easier for us to switch to another json-library if we see fit at some point. The other optional requirements socks and passport are also more a description of what the requirements are for, not which lib would be installed. So I think I'm slightly more in favor of adjusting the readme as you currently did.

@harshil21 harshil21 added the ⚙️ documentation affected functionality: documentation label Aug 13, 2021
@Bibo-Joshi Bibo-Joshi changed the base branch from master to doc-fixes August 13, 2021 14:20
@Bibo-Joshi Bibo-Joshi merged commit 5b18805 into python-telegram-bot:doc-fixes Aug 13, 2021
@Bibo-Joshi
Copy link
Member

Thanks for the contribution!

@Behoston Behoston deleted the readme_fix branch August 14, 2021 08:14
@github-actions github-actions bot locked and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚙️ documentation affected functionality: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants