search

kworkflow / patch-hub

patch-hub is a TUI that streamlines the interaction of Linux developers with patches archived on lore.kernel.org
GNU General Public License v2.0
8 stars 6 forks source link

refactor: move all Lore specific modules to new `lore` lib module #70

Closed davidbtadokoro closed 3 weeks ago

davidbtadokoro commented 4 weeks ago

[What]

Create a module named lore in the library crate to house all domain-specific modules. This considerably cleans the src/ directory, with the only con of adding another nesting level to the library crate and having to collaterally update a handful of imports.

[Why/Context]

Currently, the src/ directory of the project is a mess as it is crowded with the modules defined in the library crate and the binary crate as well. Specifically, the modules from the lib crate (defined in lib.rs) are domain-specific, i.e., they represent implementations that are significantly or entirely coupled with the Lore objects and API.

In this sense, we could extract every module from the library crate (at the moment, they all live directly in src/) to a directory lib/. This would result in a good clean-up, as all directories of these modules, like lore_api_client/, lore_session/, etc., would be encapsulated inside lib/, but the modules themselves (lore_api_client.rs, lore_session.rs), would still need to be directly under src/.

Volume Comparison

Below are two screenshots: before src/ before the refactor (left) and after the refactor (right). Note that we have to add two entries in src/ for the refactor, lore.rs and lore/.

Image 1 Image 2
davidbtadokoro commented 4 weeks ago

@OJarrisonn, @th-duvanel, @lorenzoberts, and/or @gabrielsrd, could you guys give me your opinions on this? The problem at hand (the messy src/ directory) is undeniable, IMHO, but I don't know if the solution I took is the best one...

OJarrisonn commented 4 weeks ago

I really like what you've done. Grouping those files in this lore module (that could potentially become a separate crate, since it is completely decoupled from patch-hub)

davidbtadokoro commented 3 weeks ago

I really like what you've done. Grouping those files in this lore module (that could potentially become a separate crate, since it is completely decoupled from patch-hub)

Thanks for the time and the feedback! I am going to sit this PR a little bit more as it changes a lot of the structure, but, considering your response, I will probably merge it soon.

davidbtadokoro commented 3 weeks ago

Unrelated (but not totally) discussion

In the same line as this PR, I was thinking of creating a module for the library crate that encompasses everything else we can call the "application" part of patch-hub. More specifically, a module on the same level as lore that would house the current app, handler, and ui modules. The objective is to keep the binary crate as minimal as possible with only main.rs and utils.rs. I remember reading that this is a good practice on Rust projects (sorry if I am mistaken).

I was reading about some architectural patterns that we can use in this "application" part, and one that naturally fits the MVC-inspired architecture the tool was based on is The Elm Architecture (TEA). FWIU is basically an MVC in terms of components but with messages being generated by the handler, which enables us to chain updates before refreshing the view and allows us to reason about the application more as an FSM (with the model states being the states and the messages the transitions).

In this sense, we could rename the current app and ui modules to model and view, respectively, and name this top-level module sibling to lore as app. So src would only contain lib.rs, lore/, lore.rs, app/, app.rs, utils.rs, main.rs, and test_samples/.

What do you guys think?