Glucosio / glucosio-android

Glucosio Android App
GNU General Public License v3.0
338 stars 162 forks source link

Glucose range display fixes #367

Closed riskysnail closed 6 years ago

riskysnail commented 7 years ago
coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.08%) to 16.535% when pulling 31296d8915122d68cf934e60fc696626f98b634d on riskysnail:develop into d43eb2a7089d709151c4f4f1c148d4b27059dabf on Glucosio:develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.09%) to 16.531% when pulling 097359bf2bbebacac39bead2f53182074b44459f on riskysnail:develop into d43eb2a7089d709151c4f4f1c148d4b27059dabf on Glucosio:develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.09%) to 16.531% when pulling d2d5b920158374a68de4bb2d8555aa3791a13d42 on riskysnail:develop into d43eb2a7089d709151c4f4f1c148d4b27059dabf on Glucosio:develop.

riskysnail commented 7 years ago

Do I need to do anything about the coveralls check fail?

emartynov commented 7 years ago

Sorry a lot for the delay with answer. I will try today to clean up warnings and finish the review.

Thank you for the contribution!

emartynov commented 7 years ago

Would be super nice if we write some tests for these changes.

Also, from the overall overview, the GlucoseRanges has two responsibilities now:

  1. Keep user input
  2. Constant class for ranges

It would be preferable to split them. Maybe in the future.

riskysnail commented 7 years ago

I agree about GlucoseRanges. I'm thinking about creating a new class to handle the formatting of glucose readings (auto-converting to the preferred unit, potentially adding colorblind symbols for #178). This could be a good candidate to move the colorFromReading and stringToColor functions.

piotrek1543 commented 6 years ago

@riskysnail what is status now? are you going to rearrange this code as you suggested in last commit?