Skip to content
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

Create new crate fs-utils; move Lockfile and create_parent_dir #1334

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

gefjon
Copy link
Contributor

@gefjon gefjon commented Jun 4, 2024

Description of Changes

The snapshot crate will need to create lockfiles. Rather than duplicating code to do so, we choose to move our definition of Lockfile into a crate that can be depended on by both cli and snapshot. No existing crate seems like an obvious choice for this -- a Lockfile is not really a data structure, so data-structures seems wrong -- so we add a new crate, fs-utils. Currently this contains only Lockfile and create_parent_dir, but a follow-up PR will add DirTrie, a Git-like on-disk object store.

API and ABI breaking changes

N/a

Expected complexity level and risk

1

Testing

Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!

  • CI is gonna run smoketests.

The snapshot crate will need to create lockfiles.
Rather than duplicating code to do so, we choose to move our definition of `Lockfile`
into a crate that can be depended on by both `cli` and `snapshot`.
No existing crate seems like an obvious choice for this
-- a `Lockfile` is not really a data structure, so `data-structures` seems wrong --
so we add a new crate, `fs-utils`.
Currently this contains only `Lockfile` and `create_parent_dir`,
but a follow-up PR will add `DirTrie`, a Git-like on-disk object store.
@gefjon gefjon requested a review from bfops June 4, 2024 13:19
@gefjon gefjon mentioned this pull request Jun 4, 2024
2 tasks
crates/fs-utils/src/lockfile.rs Outdated Show resolved Hide resolved
crates/fs-utils/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

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

LGTM! I really like this isolated version.

@gefjon gefjon enabled auto-merge June 4, 2024 16:32
@gefjon gefjon added this pull request to the merge queue Jun 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 4, 2024
@gefjon gefjon added this pull request to the merge queue Jun 4, 2024
Merged via the queue into master with commit db34ff6 Jun 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants