-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds basic crate-level documentation #490
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #490 +/- ##
==========================================
- Coverage 89.60% 89.59% -0.01%
==========================================
Files 76 76
Lines 13888 13894 +6
==========================================
+ Hits 12444 12448 +4
- Misses 1444 1446 +2
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺️ PR tour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺️ cargo doc
pointed out some nits in this file and the next.
fn from(value: Vec<u8>) -> Self { | ||
Value::Blob(value) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺️ While writing the doc tests, I realized we didn't have an Into<Value>
impl for the blob
-ish Rust types.
#[doc(inline)] | ||
pub use data_source::IonDataSource; | ||
#[doc(inline)] | ||
pub use raw_symbol_token::RawSymbolToken; | ||
#[doc(inline)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺️ #[doc(inline)]
prevents these from showing up in a big "Re-exports" list in the docs. We'll probably add this to more types before 1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few minor comments otherwise looks good!
Really nice to have those macro examples 🚀
|
||
use element::owned::Element; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clippy suggests this unused import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clippy says its unused, but it's actually needed for all of the [Element]
type references in the doc comments. I've added an annotation to suppress the clippy warning.
//! let mut reader = ReaderBuilder::default().build(ion_file)?; | ||
//! // A simple pretty-printer | ||
//! for element in reader.elements() { | ||
//! println!("{}", element?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a sample file content and output for this example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does stand out as the only example without an assert_eq!
, but I figured seeing how to access the elements in a file would be enough for folks to use the other tests' examples. I'm going to leave this as-is for the moment to get v0.16.0 out, but I'm open to changing it in the future.
Provides examples for creating, reading, and writing
Element
s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.