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

Update data.py to be compatible with example #1213

Merged
merged 2 commits into from Sep 26, 2018

Conversation

@reablaz
Copy link
Contributor

reablaz commented Sep 8, 2018

for now, if you process personal_info with example code, then you got an error if there is no set option to get native fist and last name.

setting default value will allow to process personal_info without native name/surname transation

for now, if you process personal_info with example code, then you got an error if there is no set option to get native fist and last name.

setting default value will allow to process personal_info without native name/surname transation
@jsmnbom

This comment has been minimized.

Copy link
Member

jsmnbom commented Sep 12, 2018

Thanks a lot for the PR @reablaz. You are indeed correct that the native arguments should be optional.

It seems like you didn't quite set up the development environment as described in .github/CONTRIBUTING.rst. Most specifically the pre-commit-hook flake8, which is complaining that

telegram/passport/data.py:42:100: E501 line too long (105 > 99 characters)

Could we perhaps get you to fix that line? :D

@jsmnbom

This comment has been minimized.

Copy link
Member

jsmnbom commented Sep 21, 2018

You still with us @reablaz ? :D

i hope i understood right this. sorry for delay, just starting using github!
@reablaz

This comment has been minimized.

Copy link
Contributor Author

reablaz commented Sep 21, 2018

Hi there! I shortened this line, thank you for your pointing!

@reablaz

This comment has been minimized.

Copy link
Contributor Author

reablaz commented Sep 22, 2018

failed again.. can anyone teach me how to do things right? :D

@jsmnbom

This comment has been minimized.

Copy link
Member

jsmnbom commented Sep 25, 2018

Actually it seems the build failed for other reasons, so this looks good :D

Would you like to add yourself to AUTHORS.rst, please do so :) Otherwise just reply and we'll get this merged :D

@reablaz

This comment has been minimized.

Copy link
Contributor Author

reablaz commented Sep 26, 2018

Hey there!

I think my contribution is too trivial to add myself to authors list. So we can skip this step. Let's merge 👍

@jsmnbom

This comment has been minimized.

Copy link
Member

jsmnbom commented Sep 26, 2018

Alright, thanks once again :D

@jsmnbom jsmnbom merged commit c714a17 into python-telegram-bot:master Sep 26, 2018
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
Hound No violations found. Woof!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.