Skip to content

Conversation

@CLiu13
Copy link
Collaborator

@CLiu13 CLiu13 commented Jan 19, 2019

Closes #107

Before raising the PR, here is a check list:

  1. have you written unit tests for your code changes?
  2. have you run "make format"?
  3. are you requesting to "dev"?
  4. have you updated the change log?
  5. do you think that you can understand your changes after 6 month?
    5.1) can someone else understand your changes without your explanation?
  6. are you pround of your code changes?
    6.1) do you have the feeling of achievement?
  7. please add your name and github link to contributors.rst in alphabetical order.

@CLiu13 CLiu13 requested review from chfw and jayvdb January 19, 2019 21:38
chfw
chfw previously requested changes Jan 20, 2019
Copy link
Member

@chfw chfw left a comment

Choose a reason for hiding this comment

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

A unit test case please!

@chfw
Copy link
Member

chfw commented Jan 20, 2019

I will look at the unit test failure.

@CLiu13
Copy link
Collaborator Author

CLiu13 commented Jan 20, 2019

I will look at the unit test failure.

Thanks. Not sure why it is occurring. New unit test case added for this change.

@jayvdb jayvdb changed the title ✨ Add -v to show current moban version WIP: ✨ Add -v to show current moban version Jan 21, 2019
@jayvdb jayvdb removed their request for review January 21, 2019 01:24
@codecov-io
Copy link

codecov-io commented Jan 22, 2019

Codecov Report

Merging #179 into dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #179      +/-   ##
==========================================
+ Coverage   97.82%   97.83%   +<.01%     
==========================================
  Files          40       40              
  Lines        2071     2079       +8     
==========================================
+ Hits         2026     2034       +8     
  Misses         45       45
Impacted Files Coverage Δ
moban/main.py 100% <100%> (ø) ⬆️
...sts/integration_tests/test_command_line_options.py 98.34% <100%> (+0.03%) ⬆️
moban/constants.py 100% <100%> (ø) ⬆️

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 986d545...a309bf7. Read the comment docs.

Copy link
Member

@chfw chfw left a comment

Choose a reason for hiding this comment

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

One more thing: update change log :)

@CLiu13 CLiu13 changed the title WIP: ✨ Add -v to show current moban version ✨ Add -v to show current moban version Jan 22, 2019
@CLiu13 CLiu13 dismissed chfw’s stale review January 22, 2019 23:40

Changes have addressed this review

@CLiu13
Copy link
Collaborator Author

CLiu13 commented Jan 22, 2019

One more thing: update change log :)

Done!

Copy link
Member

@chfw chfw left a comment

Choose a reason for hiding this comment

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

GTG

@chfw chfw merged commit 17c1a1b into moremoban:dev Jan 23, 2019
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