-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Foundations of API Design chapter #2994
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -436,6 +436,39 @@ | |
| # Idiomatic Rust | ||
|
|
||
| - [Welcome](idiomatic/welcome.md) | ||
| - [Foundations of API Design](idiomatic/foundations-api-design.md) | ||
| - [Meaningful Doc Comments](idiomatic/foundations-api-design/meaningful-doc-comments.md) | ||
| - [Avoid Redundancy](idiomatic/foundations-api-design/meaningful-doc-comments/avoid-redundancy.md) | ||
| - [Name Drop and Signpost](idiomatic/foundations-api-design/meaningful-doc-comments/name-drop-signpost.md) | ||
| - [Name and Signature are Not Enough](idiomatic/foundations-api-design/meaningful-doc-comments/what-isnt-docs.md) | ||
| - [What and Why, not How and Where](idiomatic/foundations-api-design/meaningful-doc-comments/what-why-not-how-where.md) | ||
| - [Who Are You Writing For?](idiomatic/foundations-api-design/meaningful-doc-comments/who-are-you-writing-for.md) | ||
| - [Predictable API](idiomatic/foundations-api-design/predictable-api.md) | ||
| - [Naming conventions](idiomatic/foundations-api-design/predictable-api/naming-conventions.md) | ||
| - [Get](idiomatic/foundations-api-design/predictable-api/naming-conventions/01-get.md) | ||
| - [Push](idiomatic/foundations-api-design/predictable-api/naming-conventions/02-push.md) | ||
| - [Is](idiomatic/foundations-api-design/predictable-api/naming-conventions/03-is.md) | ||
| - [Mut](idiomatic/foundations-api-design/predictable-api/naming-conventions/04-mut.md) | ||
| - [Try](idiomatic/foundations-api-design/predictable-api/naming-conventions/05-try.md) | ||
| - [With](idiomatic/foundations-api-design/predictable-api/naming-conventions/06-with.md) | ||
| - [From](idiomatic/foundations-api-design/predictable-api/naming-conventions/07-from.md) | ||
| - [Into](idiomatic/foundations-api-design/predictable-api/naming-conventions/08-into.md) | ||
| - [Owned](idiomatic/foundations-api-design/predictable-api/naming-conventions/09-owned.md) | ||
| - [By](idiomatic/foundations-api-design/predictable-api/naming-conventions/10-by.md) | ||
| - [To](idiomatic/foundations-api-design/predictable-api/naming-conventions/12-to.md) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "11-unchecked.md" is missing. |
||
| - [As and Ref](idiomatic/foundations-api-design/predictable-api/naming-conventions/13-as-and-ref.md) | ||
| - [Mini Exercise](idiomatic/foundations-api-design/predictable-api/naming-conventions/14-mini-exercise.md) | ||
| - [Implementing Common Traits](idiomatic/foundations-api-design/predictable-api/common-traits.md) | ||
| - [Debug](idiomatic/foundations-api-design/predictable-api/common-traits/01-debug.md) | ||
| - [PartialEq and Eq](idiomatic/foundations-api-design/predictable-api/common-traits/02-partialeq-eq.md) | ||
| - [PartialOrd and Ord](idiomatic/foundations-api-design/predictable-api/common-traits/03-partialord-ord.md) | ||
| - [Hash](idiomatic/foundations-api-design/predictable-api/common-traits/04-hash.md) | ||
| - [Clone](idiomatic/foundations-api-design/predictable-api/common-traits/05-clone.md) | ||
| - [Copy](idiomatic/foundations-api-design/predictable-api/common-traits/06-copy.md) | ||
| - [Serialize and Deserialize](idiomatic/foundations-api-design/predictable-api/common-traits/07-serde.md) | ||
| - [From and Into](idiomatic/foundations-api-design/predictable-api/common-traits/08-from-into.md) | ||
| - [TryFrom and TryInto](idiomatic/foundations-api-design/predictable-api/common-traits/09-try-from-into.md) | ||
| - [Display](idiomatic/foundations-api-design/predictable-api/common-traits/10-display.md) | ||
| - [Leveraging the Type System](idiomatic/leveraging-the-type-system.md) | ||
| - [Newtype Pattern](idiomatic/leveraging-the-type-system/newtype-pattern.md) | ||
| - [Semantic Confusion](idiomatic/leveraging-the-type-system/newtype-pattern/semantic-confusion.md) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| --- | ||
| minutes: 2 | ||
| --- | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you intend to add something here? At the very least, a title and |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,23 @@ | ||||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||||
| minutes: 5 | ||||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Meaningful Doc Comments | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ```rust,compile_fail | ||||||||||||||||||||||||||||||||||
| /// API for Domain // ❌ | ||||||||||||||||||||||||||||||||||
| pub mod domain {} | ||||||||||||||||||||||||||||||||||
| /// Function from A to B // ❌ | ||||||||||||||||||||||||||||||||||
| fn a_to_b(a: A) -> B {...} | ||||||||||||||||||||||||||||||||||
| /// Does X // ❌ | ||||||||||||||||||||||||||||||||||
| fn do_x() {} | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+15
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Doc comments are the most common source of documentation most developers will | ||||||||||||||||||||||||||||||||||
| engage with. | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+19
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Conciseness |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| It's important to write doc comments that developers will appreciate reading, | ||||||||||||||||||||||||||||||||||
| that gives them the information they are looking for and doesn't just re-state | ||||||||||||||||||||||||||||||||||
| the obvious. | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+23
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
More concise wording; more actionable ("information they are looking for" - not very actionable); more common spelling ("restate");.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add some framing for the instructor (in the speaker notes).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talking about Rust specifics, we didn't cover Rust's comment structure conventions. I don't want to rehash the syntactic details of Markdown, but WDYT about this?
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,55 @@ | ||||||||||
| --- | ||||||||||
| minutes: 15 | ||||||||||
| --- | ||||||||||
|
|
||||||||||
| # Avoiding Redundancy | ||||||||||
|
|
||||||||||
| Function names and type signatures already document some information, avoid | ||||||||||
| repeating them! | ||||||||||
|
Comment on lines
+7
to
+8
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| ```rust | ||||||||||
| // Don't do this! | ||||||||||
| /// Parses an ipv4 from a str. Returns an option for failure modes. | ||||||||||
| fn parse_ip_addr_v4(input: &str) -> Option<IpAddrV4> { ... } | ||||||||||
|
|
||||||||||
| // TODO: couple more of these, for the instructor to go through with students. | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My favorite:
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One specific anti-pattern to highlight is starting the comment with the item name. It is encouraged in some other ecosystems (I believe, Go), so some people might bring it over with them. /// `ServerSynchronizer` is an orchestrator that sends local edits [...]
struct ServerSynchronizer { ... }
// good
/// Sends local edits [...]
struct ServerSynchronizer { ... }/// `sync_to_server` sends local edits [...]
fn sync_to_server(...)
// good
/// Sends local edits [...]
fn sync_to_server(...) |
||||||||||
| ``` | ||||||||||
|
|
||||||||||
| <details> | ||||||||||
| - Motivation: Documentation that repeats name/signature information provides nothing new to the API user. | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Second motivation: Documentation that repeats API signatures can go out of date, while the API signatures are the source of truth. |
||||||||||
|
|
||||||||||
| - This is an understandable pattern to fall into! | ||||||||||
|
|
||||||||||
| Naive approach to "always document your code," follows this advice literally | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding the following nuance somewhere. In Google's style guide for Rust we talk about a distinction between library code and application code:
I think this distinction also applies here. Here's my quick attempt (could be a standalone slide):
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about calling out the |
||||||||||
| but does not follow the intent. | ||||||||||
|
|
||||||||||
| Tests might enforce documentation coverage, this kind of documentation is an | ||||||||||
| easy fix. | ||||||||||
|
|
||||||||||
| - The name of a function or type is part of the documentation of that function | ||||||||||
| or type. | ||||||||||
|
Comment on lines
+29
to
+30
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rust uses the word "item" to generically refer to entities. |
||||||||||
|
|
||||||||||
| Similarly, the signature of a function is part of the documentation of that | ||||||||||
| function. | ||||||||||
|
|
||||||||||
| Therefore: aspects of the subject are already covered when you start writing | ||||||||||
| doc comments! | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also: Don't be pressured to repeat information just to write a neat-looking bullet list which includes every parameter. |
||||||||||
|
|
||||||||||
| - Many areas of the standard library have minimal documentation because the name | ||||||||||
| and types do give enough information. | ||||||||||
|
|
||||||||||
| Rule of Thumb: What information is missing from a user's perspective? Other | ||||||||||
| than name, signature, and irrelevant details of the implementation. | ||||||||||
|
|
||||||||||
| - Don't drop down to language basics! Assume the reader of doc comments has an | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| intermediate understanding of the language itself, it's the API you're working | ||||||||||
| on that you're trying to document. | ||||||||||
|
Comment on lines
+45
to
+46
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| - If there is a complex topic involved with the functions and types you're | ||||||||||
| documenting, signpost to a "source of truth" if one exists such as a blog | ||||||||||
| post, an internal document, a paper etc. | ||||||||||
|
|
||||||||||
| - Collaborate with Students: Go through the methods in the slide and discuss | ||||||||||
| what might be relevant to an API user. | ||||||||||
|
|
||||||||||
| </details> | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,55 @@ | ||||||||
| --- | ||||||||
| minutes: 15 | ||||||||
| --- | ||||||||
|
|
||||||||
| # Name-dropping keywords and signposting topics | ||||||||
|
|
||||||||
| ```rust | ||||||||
| /// This function covers <namedrop>, for further reading see: reference A, B, C. | ||||||||
| fn highly_specific_function(/* */) { /* ... */ | ||||||||
| } | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's replace it with a concrete example. I was reading about the MARC format the other day. /// A parsed representation of a MARC 21 record [leader](//www.loc.gov/marc/bibliographic/bdleader.html).
///
/// A MARC leader contains metadata that dictates how to interpret the rest
/// of the record.
pub struct Leader {
/// Determines the schema and the set of valid subsequent data fields.
///
/// Encoded in byte 6 of the leader.
pub type_of_record: char,
/// Indicates whether to parse relationship fields, such as a "773 Host
/// Item Entry" for an article within a larger work.
///
/// Encoded in byte 7 of the leader.
pub bibliographic_level: char,
// ... other fields
}
/// Parses the [leader of a MARC 21 record](https://www.loc.gov/marc/bibliographic/bdleader.html).
///
/// The leader is encoded as a fixed-length 24-byte field, containing metadata
/// that determines the semantic interpretation of the rest of the record.
pub fn parse_leader(leader_bytes: &[u8; 24]) -> Result<Leader, MarcError> {
todo!()
}
#[derive(Debug)]
pub enum MarcError {}Likely most people in the audience are not familiar with library science, and the instructor could ask the audience to look for keywords in this documentation that the reader could search for to better understand what is going on ("MARC 21", "MARC record", "leader", "fields", "host item entry" etc.) |
||||||||
| ``` | ||||||||
|
|
||||||||
| <details> | ||||||||
| - Motivation: Readers of documentation will not be closely reading most of your doc comments like they would dialogue in a novel they love. | ||||||||
|
|
||||||||
| Users will most likely be skimming and scan-reading to find the part of the | ||||||||
| documentation that is relevant to whatever problem they're trying to solve in | ||||||||
| the moment. | ||||||||
|
|
||||||||
| Once a user has found a keyword or potential signpost that's relevant to them | ||||||||
| they will begin to search for context surrounding what is being documented. | ||||||||
|
|
||||||||
| - Ask the class: What do you look for in documentation? Focus on the | ||||||||
| moment-to-moment searching for information here, not general values in | ||||||||
| documentation | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
|
||||||||
| - Name-drop keywords close to the beginning of a paragraph. | ||||||||
|
|
||||||||
| This aids skimming and scanning, as the first few words of a paragraph stand | ||||||||
| out the most. | ||||||||
|
|
||||||||
| Skimming and scanning lets users quickly navigate a text, keeping keywords as | ||||||||
| close to the beginning of a paragraph as possible lets a user | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incomplete sentence. |
||||||||
|
|
||||||||
| - Signpost, but don't over-explain. | ||||||||
|
|
||||||||
| Users will not necessarily have the same domain expertise as an API designer. | ||||||||
|
|
||||||||
| If a tangential, specialist term or acronym is mentioned try to bring in | ||||||||
| enough context such that a novice could quickly do more research. | ||||||||
|
|
||||||||
| - Signposting often happens organically, consider a networking library that | ||||||||
| mentions various protocols. But when it doesn't happen organically, it can be | ||||||||
| difficult to choose what to mention. | ||||||||
|
|
||||||||
| Rule of thumb: API developers should be asking themselves "if a novice ran | ||||||||
| into what they are documenting, what sources would they look up and are there | ||||||||
| any red herrings they might end up following"? | ||||||||
|
|
||||||||
| Users should be given enough information to look up subjects on their own. | ||||||||
|
|
||||||||
| - What we've already covered, predictability of an API including the naming | ||||||||
| conventions, is a form of signposting. | ||||||||
|
Comment on lines
+52
to
+53
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
"What we've already covered" suggests that there is exactly one thing that we covered previously (or that we are referring to all of that material collectively). However we covered many things, and we are only pointing out one of them. |
||||||||
|
|
||||||||
| </details> | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| --- | ||
| minutes: 5 | ||
| --- | ||
|
|
||
| Names and Signatures are not full documentation | ||
|
|
||
| ```rust | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // bad
/// Returns a future that resolves when operation completes.
fn syncToServer() -> Future<Bool>
// good
/// Sends local edits to the server, overwriting concurrent edits
/// if any happened.
fn syncToServer() -> Future<Bool>// bad
/// Returns an error if sending the email fails.
fn send(&self, email: Email) -> Result<(), Error>
// good
/// Queues the email for background delivery and returns immediately.
/// Returns an error immediately if the email is malformed.
fn send(&self, email: Email) -> Result<(), Error> |
||
| ``` | ||
|
|
||
| <details> | ||
| - Motivation: API designers can over-commit to the idea that a function name and signature is enough documentation. | ||
|
|
||
| It's better than nothing, but it's worse than good documentation. | ||
|
|
||
| - Again, names and types are _part_ of the documentation. They are not always | ||
| the full story! | ||
|
|
||
| - TODO: give some rules of thumb for when to go into more detail, | ||
| cross-reference rust stdlib docs. This may live better in the name conventions | ||
| area. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Think about parts of the behavior that are not already covered by the function name, parameter names and types. In the example on the slide it is not obvious that
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Use comments to disambiguate. For example, consider a |
||
|
|
||
| </details> | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||
| minutes: 10 | ||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Why and What, not How and Where | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Avoid documenting irrelevant details that may frequently change. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ```rust,no_compile | ||||||||||||||||||||||||||||||||
| /// Sorts a slice. Implemented using recursive quicksort. | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh this is an interesting case study you could turn around on its head as you go through it! "This comment says that it uses quicksort. This information is irrelevant to the caller, and could change in the future. Let's leave a suggested edit in the code review to remove this part of the comment." "Our code review came back and the author is pushing back, they are saying this is actually critical for the callers to know." "Did they say why? - no - huh? I don't understand, why would the caller need to know which algorithm is used here?" "We had a quick meeting with the PR author and it turns out they were trying to signal to the caller that they should not pass in untrusted data, because quicksort can go quadratic on carefully crafted collections! https://www.cs.dartmouth.edu/~doug/mdmspe.pdf" "Wow, I didn't know that! Let's write a comment that actually communicates that." The point here is that what is implementation detail vs not depends a lot on what the actual public contract is (e.g., can you supply untrusted data or not), and that is often something that only a human can do. Doing this on this slide would dilute the message; maybe do it on a separate slide as an interactive exercise? And then also pull this point "It could be that the implementation is necessary to explain, but this is likely due to whatever effects or invariants the user of that API needs to be aware of instead." to that slide, because this example would illustrate it well. |
||||||||||||||||||||||||||||||||
| fn sort_quickly<T: Ord>(to_sort: &mut [T]) { /* ... */ | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| /// Calls an org-internal service using reqwest. | ||||||||||||||||||||||||||||||||
| fn ask_service(url: &str) -> String { /* ... */ | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+16
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Could we do this, or would rustfmt not let us?
Comment on lines
+14
to
+16
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I'm not sure if you'd want to have a discussion about the "org-internal service" part, whether it is useful or not, so let's proactively omit that. |
||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // bad
/// Fetches the resource by URL from the remote server.
///
/// This function creates a new connection, sets a timeout of 5000 ms,
/// and retries **3 times** if a 500 family error is returned, each time increasing the timeout 2x, therefore achieving exponential backoff, which is crucial for not overwhelming the remote server.
fn fetch_url(url: &str) -> Result<String, Error>
// good
/// Fetches the resource by URL.
///
/// Uses exponential backoff, configured by command line parameters X and Y.
fn fetch_url(url: &str) -> Result<String, Error>Most likely, there is some other place that is the source of truth for the exponential backoff
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // bad
/// Saves a `User` record to the Postgres database.
///
/// This function opens a new connection and begins a transaction. It checks
/// if a user with the given ID exists with a `SELECT` query. If a user is
/// not found, performs an `INSERT`.
///
/// # Errors
///
/// Returns an error if any database operation fails.
pub fn save_user(user: &User) -> Result<(), db::Error> {
// ...
}
// good
/// Atomically saves a user record.
///
/// # Errors
///
/// Returns a `db::Error::DuplicateUsername` error if the user (keyed by
/// `user.username` field) already exists.
pub fn save_user(user: &User) -> Result<(), db::Error> {
// ...
} |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| <details> | ||||||||||||||||||||||||||||||||
| - Motivation: Using doc comments to explain how a function does something internally means if that internal implementation detail changes, the doc comment needs to change as well. | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Internal information is likely irrelevant to a user. Imagine explaining in a doc | ||||||||||||||||||||||||||||||||
| comment for a function that you're using for loops to solve a problem, what is | ||||||||||||||||||||||||||||||||
| the point of this information? | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| - It could be that the implementation is necessary to explain, but this is | ||||||||||||||||||||||||||||||||
| likely due to whatever effects or invariants the user of that API needs to be | ||||||||||||||||||||||||||||||||
| aware of instead. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Focus on those effects and invariants instead of instead of the implementation | ||||||||||||||||||||||||||||||||
| details themselves. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Reiterate: Implementation details can and will change, so do not explain these | ||||||||||||||||||||||||||||||||
| details. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| TODO: Real-life example of something appropriate to a large system. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| - Don't talk about where something is used for the sake of it. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| This is another instance where this information can become stale quickly. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| - Prefer instead to focus on what the function does (though again, not how it is | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
| implemented) for a user trying to reach this practical information as quickly | ||||||||||||||||||||||||||||||||
| as possible. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| </details> | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,34 @@ | ||||||||||
| --- | ||||||||||
| minutes: 10 | ||||||||||
| --- | ||||||||||
|
|
||||||||||
| # Who are you writing for? | ||||||||||
|
|
||||||||||
| Colleagues, collaborators, largely-silent API users, or just yourself? | ||||||||||
|
|
||||||||||
| ```rust | ||||||||||
| // TODO: What's a good illustration here? | ||||||||||
| ``` | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // expert writes for experts
/// Canonicalizes the MIR for the borrow checker.
///
/// This pass ensures that all borrows conform to the NLL-Polonius constraints
/// before we proceed to MIR-to-LLVM-IR translation.
pub fn canonicalize_mir(mir: &mut Mir) {
// ...
}
// expert writes for newcomers
/// Prepares the Mid-level IR (MIR) for borrow checking.
///
/// The borrow checker operates on a simplified, "canonical" form of the MIR.
/// This function performs that transformation. It is a prerequisite for the
/// final stages of code generation.
///
/// For more about Rust's intermediate representations, see the
/// [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/mir/index.html).
pub fn canonicalize_mir(mir: &mut Mir) {
// ...
} |
||||||||||
|
|
||||||||||
| <details> | ||||||||||
| - Motivation: It can be easy to fall into a pattern of writing only for you, but most documentation is for people coming in with a different perspective. | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| - Unintentionally writing for yourself can lead to people not understanding a | ||||||||||
| point you're trying to make or the concept you're trying to articulate. | ||||||||||
|
|
||||||||||
| - Imagine a version of you, or others you've known, struggling to find practical | ||||||||||
| information while going through documentation. | ||||||||||
|
|
||||||||||
| Keep this idea of a person in mind when thinking about what areas of a | ||||||||||
| codebase need attention for doc comments. | ||||||||||
|
|
||||||||||
| - Who are you writing for? | ||||||||||
|
|
||||||||||
| - Also imagine a version of you, or others you've known, who is struggling to | ||||||||||
| find the important details in winding, extensive doc comments. Don't give too | ||||||||||
| much information. | ||||||||||
| - Always ask: Is this documentation making it difficult for the API user? Are | ||||||||||
| they able to quickly grasp what they need or find out where they could need | ||||||||||
| it? | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also say something like "Experts also read API level documentation. Doc comments might not be the right place to educate your audience about the basics of your domain. In that case, signpost and namedrop - divert people to long-form documentation." |
||||||||||
|
|
||||||||||
| </details> | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||
| --- | ||||||
| minutes: 2 | ||||||
| --- | ||||||
|
|
||||||
| # Predictable API | ||||||
|
|
||||||
| Keep your APIs predictable through naming conventions and implementing common | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Parallel sentence structure. |
||||||
| traits. | ||||||
|
|
||||||
| <!-- TODO --> | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some speaker notes would be nice. What is the framing for this section? |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| --- | ||
| minutes: 5 | ||
| --- | ||
|
|
||
| # Common Traits to Implement | ||
|
|
||
| ```rust | ||
| #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone /* ... */)] | ||
| pub struct MyData { | ||
| pub name: String, | ||
| pub number: usize, | ||
| pub data: [u8; 64], | ||
| } | ||
| ``` | ||
|
|
||
| <details> | ||
| - Traits are one of the most potent tools in the Rust language. The language and ecosystem expects you to use them, and so a big part of _predictability_ is what traits are implemented for a type! | ||
|
|
||
| - Traits should be liberally implemented on types you author, but there are | ||
| caveats! | ||
|
|
||
| - Remember, many traits have the ability to be _derived_: to have a compiler | ||
| plugin (macro) write the implementation for you! | ||
|
|
||
| - Authors of ecosystem traits (like De/Serialize) have made derive | ||
| implementations for traits available to users, leading to very little | ||
| commitment needed on the developer side for implementing these kinds of | ||
| traits! | ||
|
|
||
| </details> |
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.
I'm thinking about the structure and the order of the doc comment sections.
We provide a lot of negative advice. Starting with negative advice might look weird and might rub some people the wrong way. How about reordering "What and Why, not How and Where" and "Who Are You Writing For?" first, then the "anatomy of a doc comment" slide that I'm suggesting (see other comments), the name drop and signpost slide, and then the negative advice about redundancy?