Skip to content

Conversation

@kmvanbrunt
Copy link
Member

Removed unnecessary sorting and duplicate removal from the completers since all results from these functions are returned to complete() which already does these things.

These changes also provide better examples of what is required to write a completer and what isn't.

… since all

results from these functions are returned to complete() which already does these things.

These changes also provide better examples of what is required to write a completer and what isn't.
@kmvanbrunt kmvanbrunt requested a review from tleonhardt as a code owner March 31, 2018 05:01
@codecov
Copy link

codecov bot commented Mar 31, 2018

Codecov Report

Merging #333 into master will increase coverage by 0.3%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #333     +/-   ##
========================================
+ Coverage    89.2%   89.5%   +0.3%     
========================================
  Files           1       1             
  Lines        1917    1906     -11     
========================================
- Hits         1710    1706      -4     
+ Misses        207     200      -7
Impacted Files Coverage Δ
cmd2.py 89.5% <100%> (+0.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad463db...a6ed2bc. Read the comment docs.

tleonhardt
tleonhardt previously approved these changes Mar 31, 2018
Copy link
Member

@tleonhardt tleonhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@kmvanbrunt kmvanbrunt merged commit ad76cec into master Mar 31, 2018
@kmvanbrunt kmvanbrunt deleted the remove_extra_sorting branch March 31, 2018 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants