-
Notifications
You must be signed in to change notification settings - Fork 232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Split linutil into TUI and Core crates #274
refactor: Split linutil into TUI and Core crates #274
Conversation
32585a4
to
0280041
Compare
0280041
to
8786959
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the changes but I need to work through some of the PRs as this will break them.
Aren't there gonna be github workflows issues here? |
No. There is still only 1 binary target. |
Yes I see. I'd suggest renaming directories for:
(https://rust-lang.github.io/api-guidelines/naming.html) and removing Other than that, your PR is a very good idea. If you ask me, @ChrisTitusTech, I'd try to resolve everything and merge this ASAP. |
This is in the linutil repository, it's redundant to include it in the directory name. The crate names should include linutil since they could be published on crates.io or another similar repository. The guidelines you linked do not specify anything about matching the directory & crate's names.
I thought about that for a while, but I decided against it. The TUI will be far more actively maintained, and updating it to take advantage of features in new Rust editions would be more convenient if you don't have to also update completely functional code in another crate. Similarly, dependencies, even if overlap is present, aren't all being shared between the 2 crates unless is completely necessary. |
If linutil is going to be published on crates.io (#333), there would be two separate crates (your PR) and
Agreed. This makes sense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, a way better structure for future development!
Pull Request
Title
Split linutil into TUI and Core crates
Type of Change
Description
Restructure linutil from one crate containing all logic and scripts into two crates, along with a separate tabs directory to contain all data previously housed in src/commands/. Temporary directories are also entirely handled within the core crate, since they're not used anywhere else (since #153, script paths have been absolute).
Testing
Linutil TUI builds and functions correctly. The core crate is correctly rebuilt upon any modification to the tabs directory.
Impact
Future contributions should be simplified, since the project is structured more clearly. Additional UIs can be added (e.g. #248) without binary bloat or build flags that heavily complicate the codebase.
Despite the appearance of large changes, current PRs should be mostly compatible with this. The only major changes are to Cargo.toml files, most of everything else is just moved to other directories.
Checklist