-
Notifications
You must be signed in to change notification settings - Fork 5
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
Pe bug fixes #253
Pe bug fixes #253
Conversation
Codecov Report
@@ Coverage Diff @@
## master #253 +/- ##
=========================================
Coverage 83.23% 83.23%
Complexity 1080 1080
=========================================
Files 148 148
Lines 3179 3179
Branches 381 381
=========================================
Hits 2646 2646
Misses 442 442
Partials 91 91
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Interesting cause of the tag colour bug. I think it would be good to refactor the tag logic into its own class, that way the tag colour does not depend on the student card class either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, LGTM!
After starting Tutor's Pet:
Should I refactor the
createTag
method to make it more testable and actually test it?Resolves #235
Resolves #246