search

tresinformal / drakkar

The tresinformal video game called 'Drakkar'
GNU General Public License v3.0
11 stars 4 forks source link

End game condition check #689

Closed EvoLandEco closed 1 year ago

EvoLandEco commented 1 year ago

fixes #682

codecov-commenter commented 1 year ago

Codecov Report

Merging #689 (27c6900) into develop (5c62938) will increase coverage by 0.22%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #689      +/-   ##
===========================================
+ Coverage    86.73%   86.96%   +0.22%     
===========================================
  Files           37       37              
  Lines         1606     1634      +28     
  Branches       122      124       +2     
===========================================
+ Hits          1393     1421      +28     
  Misses         213      213
Impacted Files Coverage Δ
src/game.cpp 96.76% <100.00%> (+0.16%) :arrow_up:
src/game.h 100.00% <100.00%> (ø)
src/game_options.cpp 97.22% <100.00%> (+0.25%) :arrow_up:
src/game_options.h 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

EvoLandEco commented 1 year ago

Excellent solution! I've made a suggestion to remove the extra test you have added (I feel it's redundant with the existing one), but feel free to disagree and merge as is.

I was concerning that we might change the way we implement things later, an example is that we either implement a function outsides a class, or as a public method, I think it would be even safer to have the test to ensure that we do not accidentally change the exposed API