Result is more flexible than just the question mark

> fmt::Debug implementations should be implemented for all public types. Output will typically represent the internal state as faithfully as possible. The purpose of the Debug trait is to facilitate debugging Rust code. In most cases, using #[derive(Debug)] is sufficient and recommended.

As for more general advice, I don't have a specific reference, but the single broadest rule of thumb that I tend to follow is this: Avoid making assumptions about your callers and callees, within reason. I can also give you a bunch of special cases of this rule to help you see what it means in general:

* Where practical, prefer to take generic arguments which match some trait, rather than requiring an exact type. If the trait in question is not in std, nor in some directly-relevant library that you are compatible with, then you *may* want to define your own trait and let the caller impl it, but this is considerably more of a judgment call and I don't think a blanket rule can tell you whether it's a good idea or a bad idea.
* &T is the lowest common denominator of short-lived borrows. In theory, you could (following the first rule) insist on taking everything as Deref<T> or Borrow<Q> where T: Borrow<Q> instead of &T. In practice, it turns out that most callers really can come up with a &T if you ask for one, so the added flexibility is overkill.
* Borrow is occasionally useful if you know that your callers may want to pass a &str *and* they will want T to be String and not str in that case. This mostly only comes up with (owning) containers that offer fast key lookup, such as HashMap or any tree-like structure, but it is just about the only exception I can think of where &T is not good enough.
* Do not take &Option<T> as an argument. Always take Option<&T> instead. This is because the former requires the caller to move the T into an Option if it was not already in an Option (e.g. it's in a Box or Arc), and that's annoying. The latter can be constructed directly from &T and it is very cheap to do so because it's literally just a nullable pointer at the ABI level. A similar rule applies to Result and all other simple wrapper types.
* Do not mention smart pointers as an argument type, unless they are actually necessary. Box<T> means "I'm taking ownership of a heap-allocated T." Arc<T> means "I'm taking a long-lived borrow of a heap-allocated, reference-counted T." &Arc<T> means "I'm taking at least a short-lived borrow, but I might promote it to a long-lived borrow if I so choose," which is ever so slightly more efficient if the short-lived borrow case really happens at least some of the time. &Box<T>, Box<&T>, and Arc<&T> are all nonsense and should not be used.
* Use lifetime parameters on structs with caution. If a struct has a lifetime parameter, it will be harder for callers to reason about when it may be used and how it relates to the data which it has borrowed. This is mostly harmless on private types (or where visibility otherwise means that the caller does not have to think about it), and may be acceptable in cases where the lifetime restriction is "obvious" and straightforward. If it takes more than one or two sentences to explain, it's probably too complicated.
* Do not use type-unsafe sentinel values. Don't use integers to indicate which of several states we are in, use an enum. Don't use -1 to indicate "not found," use Option<usize> (or some other Option<T> as applicable). Etc. Generally, do not assume that the caller has carefully written out all of the if statements that your documentation says they're supposed to write - instead, make them write an exhaustive match block that cannot forget possible values.
* If you are implementing From, TryFrom, or a similar conversion trait, it's OK to mention From and Into in your where clause (to indicate that a type conversion is possible whenever some generic parameter allows a corresponding conversion). Otherwise, it may indicate that you are trying to be "too flexible." Let the caller write foo.into() if they want a type conversion to happen - do not do it for them, unless silent type conversion is appropriate for that context (which is sometimes the case, but you should think about it carefully). Note: IntoIterator is not the same thing as Into<Iterator>. It is entirely unproblematic to accept IntoIterator, because that is a collection-specific trait, rather than a generic "convert anything into anything" trait.

Another way of phrasing this: Think about how you would go about using a given API in different use cases (maybe even sketch out some example code, which can also serve as doctests). If the API is awkward or fiddly, then think about why that is the case, and whether you can find a way to make things easier for the caller.