Skip to content

Comments

refactor: Split memcached service to new crate#7023

Merged
koushiro merged 2 commits intomainfrom
extract-memcached-service
Dec 16, 2025
Merged

refactor: Split memcached service to new crate#7023
koushiro merged 2 commits intomainfrom
extract-memcached-service

Conversation

@koushiro
Copy link
Member

Which issue does this PR close?

Part of #6829

Closes #6906

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

AI Usage Statement

Zed with GPT-5.1 Codex-Max

Copilot AI review requested due to automatic review settings December 16, 2025 06:46
@koushiro koushiro requested a review from Xuanwo as a code owner December 16, 2025 06:46
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" labels Dec 16, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR successfully refactors the memcached service implementation by extracting it from the monolithic opendal-core crate into a separate opendal-service-memcached crate, following the pattern established by other recently extracted services (e.g., moka, sqlite, ipfs).

Key Changes

  • Created new opendal-service-memcached crate with its own Cargo.toml
  • Updated all import paths from crate::* to opendal_core::* throughout memcached source files
  • Removed memcached service module from opendal-core and updated feature flags
  • Added memcached as an external dependency in the facade crate's service re-exports

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
core/services/memcached/Cargo.toml New package definition for the standalone memcached service crate
core/services/memcached/src/lib.rs Updated module-level documentation and registry reference to use opendal_core::DEFAULT_OPERATOR_REGISTRY
core/services/memcached/src/*.rs Refactored imports from internal crate::* paths to external opendal_core::* paths
core/services/memcached/src/docs.md Updated documentation example to use new import paths
core/services/memcached/src/config.rs Updated test to properly propagate Result type with ? operator
core/core/src/services/mod.rs Removed memcached module declaration (now external crate)
core/core/Cargo.toml Removed services-memcached feature flag definition (fastpool still used by etcd, redis, sftp)
core/Cargo.toml Updated feature flag to use external crate dependency and fixed alphabetical ordering of service dependencies
core/src/lib.rs Added re-export of opendal_service_memcached in services module
core/Cargo.lock Added dependency entry for new opendal-service-memcached crate
core/services/sqlite/Cargo.toml Removed trailing empty line (unrelated cleanup)
Comments suppressed due to low confidence (2)

core/services/memcached/src/lib.rs:21

  • The crate-level doc comment should come after the #![cfg_attr(docsrs, feature(doc_cfg))] attribute, not before it. This follows the convention used in other service crates like moka and ipfs where attributes precede the module-level documentation comment.
    core/services/memcached/src/docs.md:31
  • The documentation example should use use anyhow::Result; instead of use opendal_core::Result; to be consistent with all other service crates (aliyun-drive, alluxio, azblob, azdls, azfile, fs, ftp, gcs, ghac, moka, mysql, obs, oss, postgresql, s3, sled, sqlite, tikv, vercel-blob).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

LGTM after CI passing

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 16, 2025
@koushiro koushiro merged commit 8eafa9e into main Dec 16, 2025
391 checks passed
@koushiro koushiro deleted the extract-memcached-service branch December 16, 2025 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[core split] Service: memcached

2 participants