Improve neo marker docs #5247
Improve neo marker docs #5247
Conversation
sffc
commented
Jul 16, 2024
sffc
commented
sffc
requested review from
Manishearth
and removed request for
zbraniecki and
nordzilla
| @@ -1057,7 +1057,7 @@ macro_rules! impl_date_marker { | |||
| input_day_of_year = $day_of_year_yesno:ident, | |||
| input_any_calendar_kind = $any_calendar_kind_yesno:ident, | |||
| ) => { | |||
| #[doc = concat!("Marker for ", $description, ": ", $expectation)] | |||
| #[doc = concat!("**“", $expectation, "**” ⇒ ", $description)] | |||
There was a problem hiding this comment.
I think using both quotation marks and bold for emphasis is overkill, and also don't understand the use of an (implication) arrow, $expectation is an example value for $description, not a cause of it.
There was a problem hiding this comment.
The quotation marks are because it is a human-readable output string as opposed to developer docs.
The boldface is to make it easier to read in a list as shown above.
The arrow was just some character I thought looked nice. Happy for suggestions on another character to use in that location.
There was a problem hiding this comment.
I do like the boldface, and I do think the quotes are contextually needed here. We could just use a colon
There was a problem hiding this comment.
I assume @robertbastian's comment was non-blocking. These are docs and we can continue to improve.
| @@ -1759,7 +1759,7 @@ impl_zone_marker!( | |||
| impl_zone_marker!( | |||
| NeoTimeZoneGenericMarker, | |||
| NeoTimeZoneSkeleton::generic(), | |||
| description = "a generic time zone format with inherited length", | |||
| description = "generic time zone with inherited length, or location if unavailable", | |||
There was a problem hiding this comment.
issue: the "with inherited length" docs use short examples, ideally they should have both long and short examples, or point to the respective markers instead.
I also don't understand from the docs where the length is inherited from.
There was a problem hiding this comment.
The docs test passes a "medium" length into the constructor, which currently resolves to short. I agree it could be more clear.
There was a problem hiding this comment.
How about "with runtime length"?
#5248 will help clarify this, too
There was a problem hiding this comment.
Not at all. I don't understand why there are both compile time and runtime lengths.
There was a problem hiding this comment.
I want to continue iterating on these docs so I would like to merge this to avoid conflicts.
There was a problem hiding this comment.
For now I'll revert back to "inherited length" since it is slightly more precise and it's the language that was there before.
| @@ -1558,7 +1558,7 @@ macro_rules! impl_zoneddatetime_marker { | |||
| impl_day_marker!( | |||
| NeoYearMonthDayMarker, | |||
| NeoDayComponents::YearMonthDay, | |||
| description = "a Year/Month/Day format", | |||
| description = "year, month, and day (era elided when possible)", | |||
There was a problem hiding this comment.
NeoNeverMarker says it's "A struct implementing traits for never loading data.", however these markers now just say "“May 17, 2024” ⇒ year, month, and day (era elided when possible)". They should say "A struct implementing traits for loading locale-depdendent date fields (e.g. “May 17, 2024”)."
Maybe the examples are better as a table in this module, because there are FullDataCalMarkers, NeoNeverMarker, and NoDataCalMarkers floating around here, which are markers but not semantic skeleta.
There was a problem hiding this comment.
Oh I definitely plan to move these markers around. I was just cleaning up the docs in the mean time. This isn't the final home of these marker types. I'm asking for a review of the markers implementing DateTimeMarkers, not a review of the other markers implementing other scaffolding traits.
