Skip to content

Mass refactor the crates structure [6 / seq 1]#200

Merged
piercefreeman merged 5 commits intomainfrom
refactor-5
Mar 2, 2026
Merged

Mass refactor the crates structure [6 / seq 1]#200
piercefreeman merged 5 commits intomainfrom
refactor-5

Conversation

@MOZGIII
Copy link
Collaborator

@MOZGIII MOZGIII commented Mar 1, 2026

This is a huge PR that I've built as a PoC for the more atomic crates; we can merge it as-is, or I can subdivide it into smaller parts so we can review individual changes.

The totality of the changes in this PR are:

  • moving the files around
  • correcting use statements

There are no semantic changes to the code, or tests; in fact, most of the files remain as-is, and don't really have any associated changes at all but the use section update and new placement.


This is the majority of the work required to split out the monolithic crate structure into small and atomic crates; we have resolved all of the dependency cycle issues and establish necessary patterns to move forward.

  • We have crates that declare the traits either standalone or together with the related (core) types; in practice, these are waymark-<subsystem>-backend crates and waymark-<subsystem>-core crates.
  • There are also two special crates: waymark-runner and waymark-runner-state; that last one could've been called waymark-runner-core, and that it because it holds the core types used by waymark-core-backend
  • The backend traits are subdivided into individual crates; the backend implementations (i.e. postgres and memory) import all of those crates; subsystem driver crates (to be extracted from the runloop later) will be importing only the relevant subsystem backend: for instance, webapp doesn't need to know about the scheduler, it will only require that the backend that is passed to it implements webapp backend.
  • The immediate next step to is to finally consume the benefits - and to split out leaf build targets (binaries) into their own crates; this will enable us to finally build the binaries with individual dependencies and (if needed) feature sets - enabling precise control of what code gets compiled into the final binary. This would, technically, partially address Minimize dependencies #148 by reducing the dependencies set to the minimum needed for a particular binary.

Again, this refactor only moves code around. It does apply no changes to the type names, or fn implementations (except correcting the item paths when they're affected by code movements).

@github-actions
Copy link

github-actions bot commented Mar 1, 2026

Coverage Report

Python Coverage

Metric Coverage
Lines 75.8%
Branches 57.8%

Download HTML Report

Rust Coverage

Metric Coverage
Lines 59.6% 🔴 (-0.1%)
Branches N/A

Download HTML Report

Compared to main branch

@MOZGIII
Copy link
Collaborator Author

MOZGIII commented Mar 1, 2026

The difference in the coverage is likely because I've moved the fault-injecting backend into a separate crate (it seems to be a reasonable thing to build up and integrate) - but now it adds to the code that needs test coverage.

@MOZGIII MOZGIII requested a review from piercefreeman March 1, 2026 22:31
Base automatically changed from refactors-4 to main March 1, 2026 23:27
@MOZGIII MOZGIII marked this pull request as ready for review March 1, 2026 23:28
@@ -0,0 +1,150 @@
// The models that we use for our backends are similar to the ones that we
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit conflicted about the naming convention with these crates. But I think almost certainly core-backend and backends-core are too similar in naming convention that it will result in confusion to new contributors joining the project...

Perhaps we switch the convention for core-backend webapp-backend and scheduler-backend to instead be:

backend-interface-core
backend-interface-webapp
backend-interface-scheduler

I mentally associate these interfaces closer to the backend-* crates than to the webapp/scheduler folders which is how they group right now in IDEs.

Then grouping the core types could then become core-webapp, core-scheduler, and core-backend to prioritize a statement of who consumes them instead of what they're about (which is already readily grokable in the name itself).

Open to hearing alternatives.

Copy link
Collaborator Author

@MOZGIII MOZGIII Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also what I had in mind - I just wanted to address this a bit later in the pipeline, together with other changes to the structure. But since this PR is the one that is introducing the crates I see how it makes sense to right away correct it here.

That said, the pattern that we are moving towards here will also eventually be well-structured; currently it seems like they are just scattered, but that's only becase we have to unwrap the logic in layers - starting from the backends.

What I'm saying is, my plan is to have a structure like this in the end: webapp-backend, webapp-core, webapp-server, webapp-config and webapp-bringup, and similarly for other subsystems (i.e. garbage-collection-task and garbage-collection-backend). The key idea here is what we definitely have certain subsystems, but not all subsystem are structured in the same way - meaning it is possible that we'd have, like, one or two -servers, couple or more -controls and etc. Maybe some unique suffixes even. Grouping things by subsystem allows us to have semantic groups even for things that would otherwise be singular.

So, to sum it up - what you are proposing makes sense now, but if we take the process further I feel like this layout will be less optimal.

@@ -11,4 +11,4 @@ uuid = { workspace = true, features = ["serde", "v4"] }
waymark-proto = { workspace = true, features = ["serde"] }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This DAG create feels a bit too light in substance, would it make sense to combine with another one or is there a dependency/feature reason why it's split independently?

@@ -0,0 +1,8 @@
[package]
name = "waymark-test-support"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the proposal above, consider switching these to semantically group support-test and support-integration

@piercefreeman piercefreeman merged commit 72dbd28 into main Mar 2, 2026
42 of 48 checks passed
@piercefreeman piercefreeman deleted the refactor-5 branch March 2, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants