Closed sharifhsn closed 8 months ago
Hi @sharifhsn ,
Thank you for this PR, you have organized the code really well! So much less clutter - appreciate your work. Adding comments for macros is nice touch!
Factoring out modules in impls
I think this is fine, otherwise it may get very fragmented. This can be revisited later if required.
I maintained a flat namespace for the types using pub use to reduce churn. However, now that the types are properly modularized, I think it would be an improvement to remove pub use and access the types using module names instead. What do you think?
I think flat namespace should be fine for now without over-modularizing.
A few data structures are shared between chat, edit, and completion. As the latter two are deprecated and unlikely to be updated, I moved those data structures into chat to better express their future usage. Should they remain, or should they be moved into a common module?
Your judgement here is reasonable - going with the current APIs and future usage and not with deprecated ones. Looks good to me in this PR
Fixes #136
This PR organizes the types for each API endpoint into their own modules, matching the module structure in the base namespace.
Work that remains:
impls
Some open questions:
I maintained a flat namespace for the types using
pub use
to reduce churn. However, now that the types are properly modularized, I think it would be an improvement to removepub use
and access the types using module names instead. What do you think?A few data structures are shared between
chat
,edit
, andcompletion
. As the latter two are deprecated and unlikely to be updated, I moved those data structures intochat
to better express their future usage. Should they remain, or should they be moved into acommon
module?