piston icon indicating copy to clipboard operation
piston copied to clipboard

An effort to improve documentation across the Piston ecosystem.

Open mitchmindtree opened this issue 8 years ago • 8 comments

@Gankro mentioned this in this reddit thread:

Admittedly, Piston has a serious documentation issue beyond unwraps in examples. I seem to recall finding some unwraps and being unable to identify what could cause them to fail (because the docs are crazy). At that point, I'm not really empowered to do anything other than leave the unwrap in, or map it to my application's equivalent of "something bad, I dunno!".

I haven't had a good look myself yet, but it might be a nice idea to scan through the crates and see where we can improve! Some ideas:

  • If a function/method returns a Result explain when and why it might return an Err.
  • If a function/method might panic! explain when and why it might do so. Even just something like:
/// This function does this and that.
///
/// **Panics** if X happens.
pub fn this_and_that() { ... }
  • Add the #![deny(missing_docs)] lint to all crates. This has been a great help in conrod and forces the developer to take the time to explain themselves. For foo-sys crates (low level C bindings) this probably isn't necessary, but I think it would be a good idea for high-level binding APIs to do this and perhaps link to the original documentation if possible (here's an example of how we do it in coreaudio-rs).
  • For low-level and high-performance crates, attempt to document the complexity and cost of the function (perhaps using big O notation where practical).
  • Add step-by-step comments to examples to explain why you're doing what you're doing, what other kinds of options there are, etc.
  • Write a guide for each crate (whether it's just a well commented example, a separate markdown file or a link elsewhere).
  • Ensure the README contains a link to the rendered docs.

I think this could benefit us greatly by making it easier for contributors to approach, contribute and maintain libraries while reducing the number of issues that are posted due to unexplained things.

mitchmindtree avatar Nov 30 '15 08:11 mitchmindtree

Another idea is to try and link to other types/methods/traits/etc when you reference them within comments, i.e. you can link to another type in the same module like this

/// Blah blah blah and it uses a [**Foo**](./struct.Foo) internally to do this.
///
/// If you need such and such, [**Bar::other_method**](./struct.Bar#method.other_method) might be a better alternative.

mitchmindtree avatar Nov 30 '15 08:11 mitchmindtree

Part of the problem is that everything is really generic and it's unclear how anything has anything. For instance, if you look at any PistonWindow example

You will see e.draw_2d, e.update, etc being invoked. e is a PistonWindow, so I would expect to find update in its docs: http://docs.piston.rs/piston_window/piston_window/struct.PistonWindow.html

Nope. Try to find how a PistonWindow has an update method, because I still am honestly unclear.

Gankra avatar Nov 30 '15 13:11 Gankra

In my experience, requiring documentation on everything tends to produce documentation of the form

/// This function does this and that. pub fn this_and_that() {

To the point where people who get used to it start needlessly transliterating function declarations even in codebases that don't require it.

Unfortunately, I don't think there's any lint to require useful documentation.

On Mon, Nov 30, 2015 at 2:29 AM, Mitchell Nordine [email protected] wrote:

@Gankro https://github.com/Gankro mentioned this in this reddit thread https://en.reddit.com/r/rust/comments/3ur9co/announcing_diesel_a_safe_extensible_orm_and_query/cxhhow2 :

Admittedly, Piston has a serious documentation issue beyond unwraps in examples. I seem to recall finding some unwraps and being unable to identify what could cause them to fail (because the docs are crazy). At that point, I'm not really empowered to do anything other than leave the unwrap in, or map it to my application's equivalent of "something bad, I dunno!".

I haven't had a good look myself yet, but it might be a nice idea to scan through the crates and see where we can improve! Some ideas:

  • If a function/method returns a Result explain when and why it might return an Err.
  • If a function/method might panic! explain when and why it might do so. Even just something like:

/// This function does this and that.////// Panics if X happens.pub fn this_and_that() { ... }

  • Add the #![deny(missing_docs)] lint to all crates. This has been a great help in conrod and forces the developer to take the time to explain themselves. For foo-sys crates (low level C bindings) this probably isn't necessary, but I think it would be a good idea for high-level binding APIs to do this and perhaps link to the original documentation if possible (here's an example https://github.com/RustAudio/coreaudio-rs/blob/master/src/audio_unit/audio_format.rs#L336 of how we do it in coreaudio-rs).
  • For low-level and high-performance crates, attempt to document the complexity and cost of the function (perhaps using big O notation where practical).
  • Add step-by-step comments to examples to explain why you're doing what you're doing, what other kinds of options there are, etc.
  • Write a guide for each crate (whether it's just a well commented example, a separate markdown file or a link elsewhere).
  • Ensure the README contains a link to the rendered docs.

I think this could benefit us greatly by making it easier for contributors to approach, contribute and maintain libraries while reducing the number of issues that are posted due to unexplained things.

— Reply to this email directly or view it on GitHub https://github.com/PistonDevelopers/piston/issues/1013.

aij avatar Dec 01 '15 03:12 aij

@Gankro See https://github.com/PistonDevelopers/piston-examples. All event traits are implemented on top of GenericEvent, so you import UpdateEvent and then use:

if let Some(args) = e.update_args() {
    ...
}

bvssvni avatar Dec 01 '15 16:12 bvssvni

@aij Good point! Still, the docs are bad in many places so we'll have to work on it.

Perhaps we should write a wiki page that describes how to write a good docs?

bvssvni avatar Dec 01 '15 16:12 bvssvni

I think there is a problem with understanding how the libraries work from just looking at the generic functions. We could add module level documentation to explain how things are connected.

bvssvni avatar Dec 22 '15 17:12 bvssvni

I have an idea: What if we make a policy that every PR should improve docs at least a little if possible? We could have a "whitelist" of the ones with good enough docs, but by default all PR should improve the docs.

bvssvni avatar Feb 22 '16 18:02 bvssvni

I've thought about writing docs before since their absence is the biggest pain point for me using Piston right now. Two things have given me pause:

  1. I am not an expert in any part of Piston - otherwise I wouldn't hate not having docs so much! Incorrect docs are worse than no docs at all, and I don't want to expect someone who is an expert to review everything I write since that may end up being more work than if you just wrote the docs yourself. Admittedly, that would become less and less of a concern as I (or whomever) get to know the code better.
  2. It seems like every 4 months there's a sea change in either Piston or one of its dependencies (glium, gfx-rs) which would make many of the docs obsolete. I'm not saying that this is a bad thing, the fast pace of development in the Rust gamedev community is quite exciting. It just seems silly to me to write documentation for something that won't exist in a year, and anyone making big changes shouldn't feel obligated to update loads of documentation at the same time.

mattico avatar Mar 09 '16 21:03 mattico

Closing.

bvssvni avatar Apr 30 '24 17:04 bvssvni