Skip to content

Conversation

@cgwalters
Copy link
Collaborator

A while ago we added a trait in preparation for multiple backends. The current composefs branch ignores it and has a bunch of if {} else {}.

Looking at this, what I think will work better in the end is actually a more refined version of the if {} else {} model instead of trying to really flesh out this trait. It's hard to say of course until we get there, but the trait approach forces a high level of abstraction vs just trying to factor out common code between two backends.

@cgwalters cgwalters requested a review from jeckersb August 15, 2025 07:33
A while ago we added a trait in preparation for multiple backends.
The current composefs branch ignores it and has a bunch of
`if {} else {}`.

Looking at this, what I think will work better in the end is
actually a more refined version of the `if {} else {}` model
instead of trying to really flesh out this trait. It's
hard to say of course until we get there, but the trait
approach forces a high level of abstraction vs just trying
to factor out common code between two backends.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters cgwalters force-pushed the drop-store-abstraction branch from 5516525 to 82057f8 Compare August 15, 2025 07:33
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a refactoring that removes the dynamic abstraction for storage backends, specifically the ContainerImageStore and ContainerImageStoreImpl traits. The logic from the sole implementation, OstreeContainerStore, has been moved into crates/lib/src/status.rs, and the crates/lib/src/store/ostree_container.rs file has been deleted. This change simplifies the codebase by removing a layer of indirection that is no longer deemed necessary, in line with the reasoning provided in the pull request description. The changes are clean, well-executed, and improve code clarity. I have no further comments or suggestions.

@cgwalters cgwalters enabled auto-merge August 15, 2025 14:05
Copy link
Collaborator

@jeckersb jeckersb left a comment

Choose a reason for hiding this comment

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

👍

yeah the backend thing never really panned out, this definitely makes things a lot simpler

@cgwalters cgwalters merged commit e99f5cc into bootc-dev:main Aug 15, 2025
27 checks passed
@cgwalters cgwalters added the area/composefs Issues related to composefs label Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/composefs Issues related to composefs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants