Sparse documentation leads to developer confusion

[frankmcsherry]

We repeatedly have issues where the scarcity of documentation leads other devs to be confused about what code is supposed to do and whether it is correct or can be changed in any particular way. The most recent example was #2639. There have been many other examples.

This is actually a problem. I (and others of you who have come to me) spend time trying to understand other people's code. That is time we all could be spending doing other things! The amount of time persons A spend trying to understand person B's code is potentially unrelated to the amount of time person B saves by not documenting their code. It can be much larger! In can be much smaller, but person B is often the wrong person to judge how little documentation their code requires!

We've already written a substantial amount of ill-documented code, and we should both 1. start writing better comments and 2. start to correct existing codebases.

[ruchirK]

IMO (and maybe this is totally a newb concern) but I find trait bounds convey a lot of semantic constraints where its really not obvious why those constraints were necessary e.g. from render/mod.rs

 474 impl<G> Context<G, RelationExpr, Row, Timestamp>
 475 where
 476     G: Scope<Timestamp = Timestamp>,

-- what kind of thing is G supposed to be? why is the trait bound on 476 important? This is the sort of thing that gets copied and pasted into a bunch of other places in the codebase as well (I am personally 100% guilty of doing a copy-paste of something like this without fully understanding what it was doing)

I'm not sure how this idiom / language structure could be better documented, just pointing out something that was a big understanding hurdle for me

[frankmcsherry]

We can certainly document it, but in this case it is because timely requires them and Rust doesn't support implied bounds yet (we can't define a new trait that imposes this constraint and have that imposition remove this requirement). This particular example looks more confusing than it needs to be because we've chosen the name Timestamp for the instantiation of the Timestamp associated type.

But perhaps what this means is that types like this should have comments about their generic arguments!

Edit: and in case it helps, the constraint is "G must implement timely's Scope trait, and must use Timestamp as its timestamp".