AnalogIO / coffeecard_app

Cross-platform coffee card app for Cafe Analog
https://www.cafeanalog.dk/app
MIT License
6 stars 1 forks source link

fix: various things #557

Closed marfavi closed 6 months ago

marfavi commented 7 months ago
ghost commented 7 months ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/AnalogIO/coffeecard_app/557/9722d927/5486555a8178c329662dc1f895ab7570f0887e87.svg)](https://app.codesee.io/r/reviews?pr=557&src=https%3A%2F%2Fgithub.com%2FAnalogIO%2Fcoffeecard_app) #### Legend CodeSee Map legend
codecov[bot] commented 7 months ago

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (9db04b1) 72.68% compared to head (5486555) 72.36%.

Files Patch % Lines
lib/features/product/presentation/functions.dart 0.00% 18 Missing :warning:
lib/core/strings.dart 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #557 +/- ## ========================================== - Coverage 72.68% 72.36% -0.32% ========================================== Files 133 133 Lines 1596 1603 +7 ========================================== Hits 1160 1160 - Misses 436 443 +7 ``` | [Flag](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/557/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/557/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO) | `72.36% <0.00%> (-0.32%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

marfavi commented 6 months ago

thats not really what i meant. The height is fine but i would like the 1.1 to be put into a variable called lineHeight

const lineHeight = 1.1; height = ....

Not sure how this improves things. The new const lineHeight = ... line works like the line above it to minimize conditional logic in the return statement. If you mean something else, can you show a complete example?

fremartini commented 6 months ago

thats not really what i meant. The height is fine but i would like the 1.1 to be put into a variable called lineHeight const lineHeight = 1.1; height = ....

Not sure how this improves things. The new const lineHeight = ... line works like the line above it to minimize conditional logic in the return statement. If you mean something else, can you show a complete example?

I pushed a commit now. I think its a code smell using a 'magic value' (1.1) in this case. This value could represent anything if you don't know its supposed to represent line height