feat(refactor): add core library foundation with internal abstractions [1 of 8]#273
feat(refactor): add core library foundation with internal abstractions [1 of 8]#273
Conversation
- Rename crate from dev-scope to dx-scope - Add src/internal module with UserInteraction and ProgressReporter traits - Add InquireInteraction implementation in cli/mod.rs - Update build.rs for library support - Update doc tests and binaries to use new crate name This establishes the library-first architecture foundation. Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
whizzzkid
force-pushed
the
feat/refactor-library-part_1
branch
from
3de01b0 to
2f73416
Compare
whizzzkid
changed the title
rubberduck203
left a comment
rubberduck203
left a comment
There was a problem hiding this comment.
Thanks for breaking this up. Much easier to review.
| @@ -1,5 +1,5 @@ | |||
| use clap::Parser; | |||
| use dev_scope::prelude::*; | |||
| use dx_scope::prelude::*; | |||
There was a problem hiding this comment.
2 questions
- Why?
- What does dx refer to here?
| #[test] | ||
| fn test_inquire_interaction_is_send_sync() { | ||
| fn assert_send_sync<T: Send + Sync>() {} | ||
| assert_send_sync::<InquireInteraction>(); |
There was a problem hiding this comment.
This test appears to be testing the compiler's type system.
I don't think we need to do that.
|
|
||
| // Re-export commonly used types at the module level | ||
| pub use progress::{NoOpProgress, ProgressReporter}; | ||
| pub use prompts::{AutoApprove, DenyAll, UserInteraction}; |
There was a problem hiding this comment.
As part of this, we should stop reexporting everything all the time.
This dumps all of these types into the internal namespace.
|
|
||
| /// Finish the current group. | ||
| fn finish_group(&self); | ||
| } |
There was a problem hiding this comment.
I actually really like this abstraction. I'm interested to see how it's used because I'm wondering if there's an auto-close implementation using the Drop trait that could automatically call finish_group for us.
| #[test] | ||
| fn test_noop_progress_is_send_sync() { | ||
| fn assert_send_sync<T: Send + Sync>() {} | ||
| assert_send_sync::<NoOpProgress>(); |
There was a problem hiding this comment.
We seem to be testing the compiler again.
| @@ -1,10 +1,14 @@ | |||
| pub mod analyze; | |||
| pub mod doctor; | |||
| pub mod internal; | |||
There was a problem hiding this comment.
I don't believe this should be pub considering it's an internal module.
| pub mod shared; | ||
|
|
||
| // CLI module is internal - not part of public library API | ||
| pub(crate) mod cli; |
There was a problem hiding this comment.
It's okay if this comes later, but ideally, we do not export cli here, even inside the library.
We should only consume cli from the executables.
I do totally understand if this is mid-refactor and we're not there yet.
|
|
||
| // Re-export internal abstractions at crate root for convenience | ||
| pub use internal::progress::{NoOpProgress, ProgressReporter}; | ||
| pub use internal::prompts::{AutoApprove, DenyAll, UserInteraction}; |
There was a problem hiding this comment.
We really need to stop dumping everything into the root namespace and preludes.
| pub use internal::prompts::{AutoApprove, DenyAll, UserInteraction}; | ||
|
|
||
| // Re-export CLI implementation for interactive applications | ||
| pub use cli::InquireInteraction; |
There was a problem hiding this comment.
This exports the type directly into the crate's root namespace.
| builder.all_build(); | ||
|
|
||
| // Git info only available when building from git repo | ||
| // This allows the crate to be built from crates.io where .git doesn't exist |
There was a problem hiding this comment.
Good comment. Says why not what. Double plus good.
2 participants
Part 1 of 8
This is 1/8 of #265
Description of Changes (auto-gen)
This establishes the library-first architecture foundation.