docs: pin the libmuslim header download and document the include-once rule - #28
Merged
Conversation
The C section documented every type and function but never said what the numbers are worth. Prayer times are an act of worship, so a page that states what has been measured and what has not belongs next to the API. accuracy.md records the measured maghrib bound of 6.5966 s against a JPL DE440 validated solver, and is equally explicit that the bound covers |latitude| <= 60 only, that sunrise, fajr, isha and asr have no oracle, and that dhuha returns NaN above roughly 62.5 degrees with no error and no sentinel. That NaN is a raw C hazard only. The Rust binding returns Error::NonFiniteResult and the Dart binding throws PrayerTimesUnavailable, so the page says so rather than implying every caller has to write the check. The overview told readers to curl the headers from main. main moves, and a vendored header changing underneath a project is the failure the single-header convention exists to avoid, so it now pins v0.1.0 and points at the releases page. It also gains the include-once caveat. The implementation block sits outside the include guard, so a file that defines the macro and then includes another header reaching prayertimes.h gets a redefinition error. This cost a real build break downstream, and a dedicated translation unit is the fix.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Not wanted. The two overview fixes stand on their own.
rizukirr
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes to the C overview, following the v0.1.0 release.
The download now pins a tag
The install instructions told readers to
curlthe headers frommain.mainmoves, and a vendored header changing underneath a project is exactly the failure the single-header convention exists to avoid.It now pins
v0.1.0and links the releases page so the next tag is one click away.Include-once caveat
The implementation block sits outside the include guard, as it does in
stbandminiaudio. So a file that definesPRAYERTIMES_IMPLEMENTATIONand then includes another header that reachesprayertimes.hgets:The header already says to define the macro in exactly one source file, which reads as sufficient but is not. It cost a real build break downstream in muslimtify, where
display.cdefined the macro and then included three headers that each reachedprayertimes.h.The documented fix is a dedicated translation unit that includes nothing else, which is what muslimtify now does and what its vendored
miniaudio.calready did.Verification
npm run buildsucceeds.