Add missing doc for segmenter, and turn on the missing_docs warning #2366
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2366 +/- ##
==========================================
- Coverage 78.21% 74.69% -3.52%
==========================================
Files 409 416 +7
Lines 33328 34964 +1636
==========================================
+ Hits 26066 26116 +50
- Misses 7262 8848 +1586
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
experimental/segmenter/src/lib.rs
Outdated
| @@ -152,6 +153,8 @@ mod line; | |||
| mod sentence; | |||
| mod word; | |||
|
|
|||
| // icu_datagen uses provider, but we don't want to expose this implementation detail to the users. | |||
| #[doc(hidden)] | |||
There was a problem hiding this comment.
Most of the provider module is already documented, and we generally keep it public in other crates, since it is effectively part of our public API. I think you can document or suppress docs on particular structs/fields.
There was a problem hiding this comment.
Will revert this hunk, and add more docs.
| @@ -113,6 +113,7 @@ pub struct LineBreakSegmenter { | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Suggestion: can you move the examples in the top-level lib.rs into each of the individual structs? Then you don't need to say "Please see the module-level documentation". This would be better, and the top-level documentation can link down to each individual segmenter struct for details.
There should still be at least one top-level example, probably for either word break or line break.
There was a problem hiding this comment.
praise: nice suggestion! Will do.
Also, run `cargo make generate-readmes` to update README.md.
| #[allow(missing_docs)] | ||
| #[cfg_attr(feature = "serde", serde(borrow))] | ||
| pub dim: ZeroVec<'data, i16>, | ||
| #[allow(missing_docs)] | ||
| #[cfg_attr(feature = "serde", serde(borrow))] | ||
| pub data: ZeroVec<'data, f32>, |
There was a problem hiding this comment.
Question: I don't know lstm well enough to document these two fields. @sffc or @makotokato Could you help on this?
There was a problem hiding this comment.
I think #[allow(missing_docs)] is okay for now.
|
@makotokato @sffc Can I have your review for this PR? |
aethanyc
added
the
waiting-on-reviewer
Yes; in the future just hit the re-request review button so it shows up in my queue. |
Ah, thank you for the tip and the review! |
aethanyc
removed
the
waiting-on-reviewer
I follow the wording of
try_newin FFI.icu4x/ffi/diplomat/src/segmenter_grapheme.rs
Line 39 in d0cb377