search

espruino / BangleApps

Bangle.js App Loader (and Apps)
https://banglejs.com/apps
MIT License
498 stars 1.17k forks source link

fix(astrocalc): handle when moon rise/set don't occur on current day #3447

Closed paul-arg closed 5 months ago

paul-arg commented 5 months ago

I noticed a bug in the Moon > Times menu.

My (very basic) understanding is that the rise and set members of the times object returned by the getMoonTimes function car actually be undefined, depending on if the moon rise or set actually occur on the current day.

This MR handles these cases.

thyttan commented 5 months ago

Looks good to me. Just bump version in metadata and changelog and I'll merge đź‘Ť

paul-arg commented 5 months ago

All good now!

thyttan commented 5 months ago

Thanks! 🙂

thyttan commented 5 months ago 

! Removed lint exemptions for 'apps/astrocalc/astrocalc-app.js' because it has been edited

✔️ Synchronized all lint exemptions

/home/runner/work/BangleApps/BangleApps/apps/astrocalc/astrocalc-app.js

  303:10  warning  'getCenterStringX' is defined but never used  no-unused-vars

  314:5   warning  'm' is assigned a value but never used        no-unused-vars

âś– 2 problems (0 errors, 2 warnings)

https://github.com/espruino/BangleApps/actions/runs/9431360748/job/25979927607#step:5:21

Sorry - just looked at the lint warnings and as said above there was an exemption removed since the file was changed. 

Would you mind removing those unused variables, or comment them out, so we get rid of the warnings? They have nothing to do with your changes but it would be neat fixing them as well.

thyttan commented 5 months ago

I fixed the warnings from the linter. Will merge now and test on my watch as soon as the development app loader shows the new version.

Thanks again!