rust-payjoin icon indicating copy to clipboard operation
rust-payjoin copied to clipboard

`cargo doc` improvements tracking issue

Open nothingmuch opened this issue 5 months ago • 5 comments

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>
  • [ ] 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
  • [ ] 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
    • [ ] 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
    • [ ] 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
    • 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
  • [ ] 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

nothingmuch avatar Jul 10 '25 17:07 nothingmuch

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 avatar Aug 26 '25 17:08 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.

DanGould avatar Aug 26 '25 17:08 DanGould

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 avatar Sep 29 '25 10:09 zealsham

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

zealsham avatar Sep 29 '25 13:09 zealsham

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.

DanGould avatar Oct 08 '25 20:10 DanGould