Skip to content

Made Location::try_new public#7775

Merged
Manishearth merged 5 commits intounicode-org:mainfrom
Ghamza-Jd:patch-1
Mar 13, 2026
Merged

Made Location::try_new public#7775
Manishearth merged 5 commits intounicode-org:mainfrom
Ghamza-Jd:patch-1

Conversation

@Ghamza-Jd
Copy link
Contributor

@Ghamza-Jd Ghamza-Jd commented Mar 12, 2026

In order to get observational islamic date for locations other than MECCA and CAIRO

Example:

let new_york = Location::try_new(40.7, -74.0, 10.0, -5.0 / 24.0).unwrap();
let fixed = fixed_from_gregorian(2026, 3, 13);
let (year, month, day) = observational_islamic_from_fixed(fixed, new_york);
println!("Observational NYC: {day}/{month}/{year}");

Changelog

calendrical_calculations: Changed Location::try_new from pub(crate) to pub

  • New Methods: Location::try_new()
  • New errors: LocationOutOfBoundsError

In order to get observational islamic date for locations other than `MECCA` and `CAIRO`
@Ghamza-Jd Ghamza-Jd requested a review from a team as a code owner March 12, 2026 23:34
@CLAassistant
Copy link

CLAassistant commented Mar 12, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

I think this is fine, but we should be careful about new public APIs

If htis is public, do we think it's sufficiently well-documented?

cc @sffc @robertbastian

@Ghamza-Jd
Copy link
Contributor Author

I've updated the docs to include utc_offset bound

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.

I'm in favor of making calendrical_calculations more useful. I've also hit situations before where constructors that "should be" public were just not public.

@Ghamza-Jd Ghamza-Jd requested a review from sffc March 13, 2026 10:20
@Manishearth Manishearth enabled auto-merge (squash) March 13, 2026 16:19
@Manishearth
Copy link
Member

Please fix the doc CI issue (ignore the semver one, I don't know why that's failing)

auto-merge was automatically disabled March 13, 2026 20:58

Head branch was pushed to by a user without write access

Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Verified that LocationOutOfBoundsError is decently written and ready to be public

@Manishearth Manishearth enabled auto-merge (squash) March 13, 2026 21:06
@Manishearth Manishearth merged commit f9a29c7 into unicode-org:main Mar 13, 2026
33 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.

4 participants