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

Propertyconstraintexception is unchecked #65

Merged

Conversation

spyros87
Copy link

@spyros87 spyros87 commented Jun 17, 2018

#63

@coveralls
Copy link

coveralls commented Jun 17, 2018

Coverage Status

Coverage increased (+0.6%) to 56.223% when pulling e7f78cf on spyros87:propertyconstraintexception-is-unchecked into c228e88 on ChargeTimeEU:master.

@spyros87 spyros87 force-pushed the propertyconstraintexception-is-unchecked branch from 5268927 to e1e566f Compare June 18, 2018 14:24
@codecov-io
Copy link

codecov-io commented Jun 18, 2018

Codecov Report

Merging #65 into master will increase coverage by 0.63%.
The diff coverage is 75.42%.

@@             Coverage Diff              @@
##             master      #65      +/-   ##
============================================
+ Coverage     52.82%   53.46%   +0.63%     
- Complexity      787      812      +25     
============================================
  Files           180      180              
  Lines          3061     3045      -16     
  Branches        221      211      -10     
============================================
+ Hits           1617     1628      +11     
+ Misses         1359     1333      -26     
+ Partials         85       84       -1

@spyros87 spyros87 force-pushed the propertyconstraintexception-is-unchecked branch from e1e566f to e7f78cf Compare June 18, 2018 14:53
@spyros87
Copy link
Author

@TVolden The PR is ready for review.

Copy link
Member

@TVolden TVolden left a comment

Choose a reason for hiding this comment

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

Good work.
I'm not a big fan of testing specific text, especially for an exception. It's subject to change and doesn't add allot of value to the test.
Other than that, everything looks fine.

@TVolden TVolden merged commit be0fbcd into ChargeTimeEU:master Jun 18, 2018
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.

4 participants