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

Convenience full_name method for User #949

Merged
merged 1 commit into from Dec 30, 2017

Conversation

Projects
None yet
4 participants
@graynk
Contributor

graynk commented Dec 24, 2017

Returns full name (first + last) if last name is present or just the first name if not present.

Small convenience method for User, which returns full name if last na…
…me is present or just the first name if not
@robert-cody

This comment has been minimized.

Show comment
Hide comment
@robert-cody

robert-cody Dec 24, 2017

In many locales format should be "surname name" (which is BTW more useful for sorting), so it's better to make locale as incoming optional parameter and return full_name depending on locale.

robert-cody commented Dec 24, 2017

In many locales format should be "surname name" (which is BTW more useful for sorting), so it's better to make locale as incoming optional parameter and return full_name depending on locale.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Dec 24, 2017

Codecov Report

Merging #949 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
- Coverage    91.8%   91.69%   -0.12%     
==========================================
  Files         103      103              
  Lines        4040     4044       +4     
  Branches      638      639       +1     
==========================================
- Hits         3709     3708       -1     
- Misses        193      197       +4     
- Partials      138      139       +1
Impacted Files Coverage Δ
telegram/user.py 86.95% <100%> (+1.24%) ⬆️
telegram/utils/request.py 66.96% <0%> (-0.9%) ⬇️
telegram/bot.py 87.51% <0%> (-0.5%) ⬇️

codecov bot commented Dec 24, 2017

Codecov Report

Merging #949 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
- Coverage    91.8%   91.69%   -0.12%     
==========================================
  Files         103      103              
  Lines        4040     4044       +4     
  Branches      638      639       +1     
==========================================
- Hits         3709     3708       -1     
- Misses        193      197       +4     
- Partials      138      139       +1
Impacted Files Coverage Δ
telegram/user.py 86.95% <100%> (+1.24%) ⬆️
telegram/utils/request.py 66.96% <0%> (-0.9%) ⬇️
telegram/bot.py 87.51% <0%> (-0.5%) ⬇️
@graynk

This comment has been minimized.

Show comment
Hide comment
@graynk

graynk Dec 24, 2017

Contributor

I don't think this information is in python locale module though?
It seems to have been in CLDR at some point, but not anymore, if I understand it correctly. In any case, I view this as just a simpler version of User.name method that works exactly the same way, only ensuring that returned names are consistent in style (I feel that mentioning some users via "@username" and others via "name" is unintuitive and doesn't look all that well in group chats).

Contributor

graynk commented Dec 24, 2017

I don't think this information is in python locale module though?
It seems to have been in CLDR at some point, but not anymore, if I understand it correctly. In any case, I view this as just a simpler version of User.name method that works exactly the same way, only ensuring that returned names are consistent in style (I feel that mentioning some users via "@username" and others via "name" is unintuitive and doesn't look all that well in group chats).

@robert-cody

This comment has been minimized.

Show comment
Hide comment
@robert-cody

robert-cody Dec 24, 2017

@graynk you're right, for some strange reason, there is no such info in Python's locale module.

robert-cody commented Dec 24, 2017

@graynk you're right, for some strange reason, there is no such info in Python's locale module.

@Eldinnie Eldinnie self-assigned this Dec 24, 2017

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Dec 24, 2017

Member

Looks good to me. Closes #943

Member

Eldinnie commented Dec 24, 2017

Looks good to me. Closes #943

@tsnoam

This comment has been minimized.

Show comment
Hide comment
@tsnoam

tsnoam Dec 30, 2017

Member

thank you for your contribution

Member

tsnoam commented Dec 30, 2017

thank you for your contribution

@tsnoam tsnoam merged commit d347c0d into python-telegram-bot:master Dec 30, 2017

5 checks passed

codeclimate All good!
Details
codecov/patch 100% of diff hit (target 91.8%)
Details
codecov/project Absolute coverage decreased by -0.11% but relative coverage increased by +8.19% compared to 3d4e050
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment