-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
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.
Thanks for this update!
As explained, test coverage is low because of the database which is not a problem, however, do you think it's worth it to refactor the duplicates?
Co-authored-by: jiabaow <[email protected]>
Co-authored-by: jiabaow <[email protected]>
Co-authored-by: jiabaow <[email protected]>
dbUserRef.child(uid).child(fieldName) | ||
.get().addOnSuccessListener { t -> | ||
var finalVal = newVal | ||
if(t.value != null) { |
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.
Identical blocks of code found in 2 locations. Consider refactoring.
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.
Do you think it is worth spending time refactoring these two duplicate functions?
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.
nice work 👍
app/src/androidTest/java/ch/sdp/vibester/activity/TypingGameActivityTest.kt
Outdated
Show resolved
Hide resolved
…tivityTest.kt Co-authored-by: Maxime <[email protected]>
Co-authored-by: Maxime <[email protected]>
Co-authored-by: Maxime <[email protected]>
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.
👍
Code Climate has analyzed commit 71b39aa and detected 8 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 44.1% (80% is the threshold). This pull request will bring the total coverage in the repository to 84.2% (0.1% change). View more on Code Climate. |
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
dbUserRef.child(uid).child(fieldName) | ||
.get().addOnSuccessListener { t -> | ||
var finalVal = newVal | ||
if(t.value != null) { |
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.
Do you think it is worth spending time refactoring these two duplicate functions?
This PR updates the score after each typing game ended based on genre/mode.
Changes:
TODO next pr: save score per lyrics game, update UI