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

feat(ovfs): supplementary opendal related functions #4825

Closed
wants to merge 4 commits into from

Conversation

zjregee
Copy link
Member

@zjregee zjregee commented Jun 30, 2024

No description provided.

@zjregee zjregee changed the title feat(ovfs): add opendal operator feat(ovfs): supplementary opendal related functions Jul 8, 2024
@zjregee
Copy link
Member Author

zjregee commented Jul 8, 2024

@Xuanwo, it's ready for review now.

struct FileKey(usize);

/// OpenedFile represents file that opened in memory.
#[allow(dead_code)]
Copy link
Member

Choose a reason for hiding this comment

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

Hi, from my perspective, adding dead_code is not ideal. If they are useful, please consider using pub use to include them.

/// Filesystem is a filesystem implementation with opendal backend,
/// and will decode and process messages from VMs.
pub struct Filesystem {}
#[allow(dead_code)]
pub struct Filesystem {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can export Filesystem out?

@zjregee
Copy link
Member Author

zjregee commented Aug 7, 2024

Hi, @Xuanwo. I think this PR may be too old and the updated code may introduce a lot of dead code because there is no clear goal for this PR. This may be a bit strange, so I will resubmit a PR tomorrow.

@zjregee zjregee closed this Aug 7, 2024
@zjregee zjregee deleted the update-ovfs branch August 7, 2024 10:53
@Xuanwo
Copy link
Member

Xuanwo commented Aug 7, 2024

Hi, @Xuanwo. I think this PR may be too old and the updated code may introduce a lot of dead code because there is no clear goal for this PR. This may be a bit strange, so I will resubmit a PR tomorrow.

Thanks and sorry for reviewing it so late.

@zjregee
Copy link
Member Author

zjregee commented Aug 7, 2024

This won't be a problem. 💌

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