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

Add caching for the rest of the islamic calendars (‫عید مبارک!!‬) #4785

Merged
merged 6 commits into from
Apr 19, 2024

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Apr 8, 2024

Fixes #3933

Uses work from #4770 on the remaining 3 calendars

عید مبارک!!

@Manishearth Manishearth requested a review from sffc as a code owner April 8, 2024 23:33
@Manishearth Manishearth requested a review from sffc April 8, 2024 23:33
@Manishearth Manishearth changed the title Add caching for the rest of the islamic calendars Add caching for the rest of the islamic calendars (عید مبارک!!) Apr 8, 2024
@robertbastian robertbastian changed the title Add caching for the rest of the islamic calendars (عید مبارک!!) Add caching for the rest of the islamic calendars (‫عید مبارک!!‬) Apr 9, 2024
components/calendar/src/islamic.rs Outdated Show resolved Hide resolved
components/calendar/src/islamic.rs Outdated Show resolved Hide resolved
components/calendar/src/islamic.rs Outdated Show resolved Hide resolved
components/calendar/src/islamic.rs Outdated Show resolved Hide resolved
components/calendar/src/islamic.rs Outdated Show resolved Hide resolved
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@Manishearth Manishearth requested a review from sffc April 11, 2024 23:48
sffc
sffc previously approved these changes Apr 12, 2024
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much improved; yeah, let's only use the precompiled data solution when we have astonomy going on. The formulas implementing islamic-tbla and islamic-civil are very straightforward.

Thought: I wonder if we could make islamic-umalqura and islamic fall back to one of the formulaic calendars for distant dates instead of carrying around the astronomy code at runtime.

components/calendar/src/islamic.rs Outdated Show resolved Hide resolved
@Manishearth Manishearth force-pushed the islamic-rest branch 3 times, most recently from 7003831 to 1ce5959 Compare April 19, 2024 20:04
@sffc
Copy link
Member

sffc commented Apr 19, 2024

error: use of deprecated associated function `icu_calendar::islamic::IslamicCivil::new_always_calculating`: Precomputation not needed for this calendar
   --> components/calendar/benches/datetime.rs:189:47
    |
189 |         icu::calendar::islamic::IslamicCivil::new_always_calculating(),
    |                                               ^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

error: use of deprecated associated function `icu_calendar::islamic::IslamicCivil::new_always_calculating`: Precomputation not needed for this calendar
   --> components/calendar/benches/datetime.rs:198:55
    |
198 |                 icu::calendar::islamic::IslamicCivil::new_always_calculating(),
    |                                                       ^^^^^^^^^^^^^^^^^^^^^^

error: use of deprecated associated function `icu_calendar::islamic::IslamicTabular::new_always_calculating`: Precomputation not needed for this calendar
   --> components/calendar/benches/datetime.rs:209:49
    |
209 |         icu::calendar::islamic::IslamicTabular::new_always_calculating(),
    |                                                 ^^^^^^^^^^^^^^^^^^^^^^

error: use of deprecated associated function `icu_calendar::islamic::IslamicTabular::new_always_calculating`: Precomputation not needed for this calendar
   --> components/calendar/benches/datetime.rs:218:57
    |
218 |                 icu::calendar::islamic::IslamicTabular::new_always_calculating(),
    |                                                         ^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `icu_calendar` (bench "datetime") due to 4 previous erro

@Manishearth Manishearth merged commit cb7e915 into unicode-org:main Apr 19, 2024
30 checks passed
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.

Loading cache data for lunar calendars
2 participants