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

Fix documentation & coding style of User.name & User.full_name #956

Merged
merged 2 commits into from Dec 30, 2017

Conversation

Projects
None yet
3 participants
@tsnoam
Member

tsnoam commented Dec 30, 2017

No description provided.

User.name & User.full_name - coding style & documentation
 - Use string `format` instead of dict comprehension.
 - Better documentation to signify the semantics difference between
   `name` and `full_name`.
@jsmnbom

This comment has been minimized.

Show comment
Hide comment
@jsmnbom

jsmnbom Dec 30, 2017

Member

Maybe note in docs that the username returned starts with '@'?

Member

jsmnbom commented Dec 30, 2017

Maybe note in docs that the username returned starts with '@'?

@tsnoam

This comment has been minimized.

Show comment
Hide comment
@tsnoam

tsnoam Dec 30, 2017

Member

@bomjacob Good idea. On it.

Member

tsnoam commented Dec 30, 2017

@bomjacob Good idea. On it.

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Dec 30, 2017

Member

@tsnoam I would remove the note.

Member

Eldinnie commented Dec 30, 2017

@tsnoam I would remove the note.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Dec 30, 2017

Codecov Report

Merging #956 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #956      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         103      103              
  Lines        4044     4042       -2     
  Branches      639      638       -1     
==========================================
- Hits         3708     3706       -2     
  Misses        197      197              
  Partials      139      139
Impacted Files Coverage Δ
telegram/user.py 86.36% <100%> (-0.6%) ⬇️

codecov bot commented Dec 30, 2017

Codecov Report

Merging #956 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #956      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         103      103              
  Lines        4044     4042       -2     
  Branches      639      638       -1     
==========================================
- Hits         3708     3706       -2     
  Misses        197      197              
  Partials      139      139
Impacted Files Coverage Δ
telegram/user.py 86.36% <100%> (-0.6%) ⬇️
Show outdated Hide outdated telegram/user.py
Show outdated Hide outdated telegram/user.py
@jsmnbom

This comment has been minimized.

Show comment
Hide comment
@jsmnbom

jsmnbom Dec 30, 2017

Member

LGTM :)

Member

jsmnbom commented Dec 30, 2017

LGTM :)

@tsnoam tsnoam merged commit a2c0431 into master Dec 30, 2017

5 checks passed

codeclimate All good!
Details
codecov/patch 100% of diff hit (target 91.69%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +8.3% compared to d347c0d
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tsnoam tsnoam deleted the user-prop-fix-semantics branch Dec 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment