Skip to content

feat: use features defined in the generated manifest#16

Merged
Bechma merged 1 commit intocyberfabric:mainfrom
Bechma:feature-consideration
Mar 24, 2026
Merged

feat: use features defined in the generated manifest#16
Bechma merged 1 commit intocyberfabric:mainfrom
Bechma:feature-consideration

Conversation

@Bechma
Copy link
Copy Markdown
Collaborator

@Bechma Bechma commented Mar 24, 2026

Summary by CodeRabbit

  • Refactor

    • Updated internal dependency feature representation to use set-based storage for improved handling and merging accuracy.
  • Tests

    • Added test coverage for enabled feature detection in dependency configurations.

Signed-off-by: Bechma <19294519+Bechma@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 01a022c7-71fb-4d4f-a708-3bb396545650

📥 Commits

Reviewing files that changed from the base of the PR and between ae427b9 and 50e2242.

📒 Files selected for processing (4)
  • crates/cli/src/common.rs
  • crates/cli/src/mod/add.rs
  • crates/module-parser/src/config.rs
  • crates/module-parser/src/metadata.rs

📝 Walkthrough

Walkthrough

The changes refactor dependency feature handling from Vec<String> to BTreeSet<String> across multiple modules. The CargoTomlDependency struct is updated to use BTreeSet for features, and all code that creates, merges, or populates dependency features is adjusted accordingly.

Changes

Cohort / File(s) Summary
Struct Definition & Serialization
crates/module-parser/src/config.rs
Updated CargoTomlDependency.features field from Vec<String> to BTreeSet<String>, including serde attributes to skip serialization when empty.
Feature Population from Metadata
crates/module-parser/src/metadata.rs
Modified get_dependencies to extract enabled features from cargo metadata's resolve graph and populate CargoTomlDependency.features. Added unit test gets_enabled_features_for_located_packages to verify feature set population for aliased dependencies.
Feature Merging Logic
crates/cli/src/mod/add.rs
Updated merge_dependency_metadata to extend existing feature sets using extend() instead of conditional pushing. Adjusted tests to use BTreeSet::from([...]) for feature construction.
Feature Mutation Operations
crates/cli/src/common.rs
Changed create_required_deps to mutate feature sets using insert() for existing dependencies rather than reassigning vectors. Updated imports to include BTreeSet.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 From vectors scattered, now sets align,
Features bundled, pristine and fine,
No duplicates hop through our store,
A BTreeSet organizes more and more! 🌳✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main feature change: dependency features are now extracted and used from the generated manifest instead of being ignored or hardcoded.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Bechma Bechma merged commit 44d9b74 into cyberfabric:main Mar 24, 2026
2 checks passed
@Bechma Bechma deleted the feature-consideration branch March 24, 2026 15:35
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.

1 participant