Skip to content

Conversation

@ihoru
Copy link
Contributor

@ihoru ihoru commented Jul 23, 2017

No description provided.

@Eldinnie
Copy link
Member

As you can see in the tests, they fail for python 2.7 (ignore the webhook timeout. And can you explain why this would be an improvement over static methods?

Calling super(ClassName, cls).de_json fix to make it work in python 2.7
@jsmnbom
Copy link
Member

jsmnbom commented Jul 23, 2017

They are an improvement since they make subclassing of our objects possible without necessarily needing to overwrite the de_json method

@ihoru
Copy link
Contributor Author

ihoru commented Jul 23, 2017

Errors fixed.

Take a look at my explanation here, please: #734

@Eldinnie
Copy link
Member

I was just wondering why it would make a difference. But seeing that tests keep up. I think it's fine.

@Eldinnie Eldinnie added the 📋 pending-merge work status: pending-merge label Jul 23, 2017
@tsnoam
Copy link
Member

tsnoam commented Jul 23, 2017

@ihoru
Thank you for your contribution

@tsnoam tsnoam merged commit 08d298e into python-telegram-bot:master Jul 23, 2017
@ihoru ihoru deleted the changing-de_json branch July 29, 2017 03:43
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📋 pending-merge work status: pending-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants