Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embed recommended high altitude rule into prayer calculations #61

Merged
merged 7 commits into from
Jun 5, 2021
Merged

Embed recommended high altitude rule into prayer calculations #61

merged 7 commits into from
Jun 5, 2021

Conversation

basememara
Copy link
Contributor

@basememara basememara commented Jun 3, 2021

Wondering what your thoughts are on baking in the high altitude recommendation into the prayer calculations. I tried to do this in a backwards compatible way so the multiple nil-coalescing is a little funky.

You can see I had to fix a unit test that was failing because of this change, but it's a good failing because this change caught a non-sense prayer times table for the UK.

This PR is in draft mode until discussed and repointed to develop branch after it gets updated.

@z3bi
Copy link
Contributor

z3bi commented Jun 5, 2021

thankfully nightPortions is not a public method so we don't really need to maintain backwards compatibility and we can make the coordinates required. That should simplify some of the unwrapping logic.

In general I think this is a good improvement 👍

@basememara basememara marked this pull request as ready for review June 5, 2021 22:44
@basememara
Copy link
Contributor Author

Sweet, ok I made the update and changed to review.

@basememara basememara changed the base branch from main to develop June 5, 2021 22:59
@z3bi z3bi merged commit fd729bb into batoulapps:develop Jun 5, 2021
@basememara basememara deleted the auto-high-alt branch June 6, 2021 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants