`cargo doc` improvements tracking issue

[nothingmuch]

Following up from #769, I went through cargo doc --no-deps --all-features, there's a few things I think we should do to improve the docs even more:

• Each typestate struct Foo in v2 should link to Receiver<Foo>
• v2 sender states (WithReplyKey, PollingForProposal, PendingFallback) should link to Sender<Foo>
• v1 should be refactored to use the typestate pattern
  • both for consistency with v2
  • and more importantly because that has the effect of showing the state machine in its logical order under Receiver struct, that alone is justification enough IMO
    • Dan's comment: one struct per state, each transition consuming self
• the top level payjoin module doc should:
  • refer to the {v1,v2}::{send,receive} modules for the actual API, without prior knowledge it's hard to tell that these are the main entry points into the API they're just in the middle of the modules list: redo docs(payjoin): highlight {v1,v2}::{send,receive} as primary API entry #1183 undone in de1a1ae
  • explain refer to payjoin-cli crate as an example
• Document the Uri top level type alias and struct
  • documentation for the individual fields exists but this assumes the reader is familiar with the Extras pattern of bitcoin_uri
    • Uri and PjUri are newtypes with their own docs, so the Extras-pattern complaint no longer applies
  • it should have an example of parsing a URI checking the version, and initializing sender state machine
• we should eliminate as many top level pub symbols as we can Keeping these. Since Clean up re-exports #688 the crate root is the only public path to them, and most appear in public signatures (OhttpKeys in ReceiverBuilder::new, HpkePublicKey from PjParam::receiver_pubkey, OhttpResponse and Request from create_*_request).
  • instead of pub use crate::core::* we should only re-export what really makes sense at the top level
  • if i'm not mistaken HpkeKeyPair, HpkePublicKey, OhttpKeys don't need to be reexported anymore at the very least
  • Url seems especially problematic since it's a re-export of a crate we're considering trying to remove, and is largely superseded by the IntoUrl trait IIUC it's its own type now
• OHTTP request reuse warning can be removed from top level send module since the v2 sub-module already contains it too (but that one lacks a level 2 heading, sorry i missed this in review)
• payjoin-cli, nolooking and bitmask-core are linked as examples in the top level v2::send module docs but:
  • this should be moved to the top level payjoin module rather than v2::send
  • afaik nolooking doesn't include v2 integration, should a caveat be added? should it be brought up to date? or is it no longer a useful example?
  • seems worth mentioning bbmobile & cake or at least payjoin-ffi?
• ///Foo -> /// Foo
• update README.md
  • link to payjoin docs
  • explain that payjoin-cli is intended to be a reference
  • Cargo-minimal.lock seems to supersede MSRV sections

[DanGould]

I added this to the payjoin-1.0 milestone because at least the payjoin doc issues must be addressed to release payjoin-1.0 imo. Once the payjoin-1.0 related checks are done we can just remove it from the milestone.

[DanGould]

Marked as good first issue since some of these items are straightforward documentation moves and links. I encourage new contributors to take ONE list item at a time if they choose to take on parts of this issue.

[zealsham]

Is the typestate pattern refactoring for V1 still something we want done ? , seeing that the current code still uses the builder pattern . if i get a go ahead i can work on that .

[zealsham]

on second note , the type state builder pattern would be more ideal

[DanGould]

Is the typestate pattern refactoring for V1 still something we want done ?

Any v1 state machinery, outside of backwards compatibility, is basically deprecated and IMO doesn't deserve further attention. Nobody is depending on it and no new software I know of wants to build on it except for demo purposes.

If you wanted to do it for fun or to get a hang of the pattern I'd review it but if you can find something others will actually depend on that might be a better use of time. I guess Yuval's original post disagrees, and I can see his argument to do it for the sake of beauty.

[Xoulomon]

@DanGould, I am planning on working on part of this issue as a first-time contributor.

Quick question though: in regards to the MSRV section on the README, since Cargo-minimal.lock is the first and main source of truth about versioning, should the MSRV section be reworded or completely taken out, given that payjoin 1.0.0 is published with rust-version 1.85 in the Cargo.toml?

[DanGould]

Updated the list to keep current

I wish to keep the MSRV section because yes it matches rust-version and is tested by Cargo-minimal.lock in CI but it shows we have a policy in a legible human-facing format. No need to claim here, if you want to take something on go for it in a PR.

The top-level crate docs (#1183's text was lost in de1a1ae) is an good first thing to tick off would be nice to point to the mods:

//! Start with [`send::v2`] and [`receive::v2`]. BIP 78 support is in `send::v1` and
//! `receive::v1` behind the `v1` feature. [payjoin-cli](https://github.com/payjoin/rust-payjoin/tree/master/payjoin-cli)
//! is a complete example of both.