AstronomicalSimulation -> ReingoldSimulation
#7342
Conversation
robertbastian
force-pushed
the
simulated
branch
from
5c8e0c3 to
99cd652
Compare
robertbastian
force-pushed
the
simulated
branch
from
99cd652 to
5ea69c0
Compare
sffc
left a comment
sffc
left a comment
There was a problem hiding this comment.
I think this direction is consistent with #7340, since it takes advantage of the bullet point "We decide to tweak the precise astronomical simulation used" and keeps the type AstronomicalSimulation as representing an astronomical simulation, just a different one than we had been shipping before.
| /// | ||
| /// This corresponds to the `"islamic-rgsa"` [CLDR calendar](https://unicode.org/reports/tr35/#UnicodeCalendarIdentifier). |
There was a problem hiding this comment.
Please say something like "there is currently no CLDR calendar that corresponds to this"
| Ok(AnyCalendarKind::HijriTabularTypeIIFriday) | ||
| } | ||
| Hijri(Some(HijriCalendarAlgorithm::Rgsa)) => Ok(AnyCalendarKind::HijriSimulatedMecca), | ||
| Hijri(Some(HijriCalendarAlgorithm::Rgsa)) => Err(()), |
There was a problem hiding this comment.
We don't return Err elsewhere in this fn; what is the impact?
There was a problem hiding this comment.
yes we do, for -u-ca-islamic
| #[non_exhaustive] | ||
| pub enum ReingoldSimulation { |
There was a problem hiding this comment.
Issue: I prefer the old model where the fields are private because we might want fields to tweak behavior besides just the location, perhaps the crescent moon criterion, or how far back to run calculations or something
There was a problem hiding this comment.
we can still change things because this is unstable. I prefer exposing one and not two unstable types
Co-authored-by: Shane F. Carr <shane@unicode.org>
2 participants
No description provided.