Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #461 +/- ##
==========================================
+ Coverage 97.73% 99.35% +1.62%
==========================================
Files 14 14
Lines 661 623 -38
Branches 177 172 -5
==========================================
- Hits 646 619 -27
+ Misses 15 4 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a major version 4 update that removes deprecated methods and refactors the attribute filtering mechanism. The changes focus on simplifying the API by removing legacy methods and replacing the attributes method override pattern with a more focused select method for filtering attributes.
Key changes include:
- Removal of deprecated inference methods (
enable_inference!,disable_inference!,inferring) - Removal of deprecated
resource_withmethod in favor ofresource_for - Replacement of
attributesmethod override pattern withselectmethod for attribute filtering - Removal of deprecated test cases for removed methods
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/alba.rb | Removes deprecated inference and resource methods |
| lib/alba/resource.rb | Removes deprecated attributes method and updates attribute processing to use @_attributes directly |
| test/usecases/nested_array_and_hash_test.rb | Updates from attributes override to select method implementation |
| test/usecases/inheritance_and_ignorance_test.rb | Updates from attributes override to select method implementation |
| test/alba_test.rb | Removes tests for deprecated methods |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| def attributes | ||
| @_attributes.except(:config) | ||
| def select(key, *) |
There was a problem hiding this comment.
The select method signature uses * to ignore additional parameters, but the method in inheritance_and_ignorance_test.rb explicitly names the second parameter as _value. Consider using a consistent signature across all implementations for better API clarity.
| def select(key, *) | |
| def select(key, _value) |
| end | ||
|
|
||
| # Default implementation for selecting attributes | ||
| # Override this method to filter attributes based on key and value |
There was a problem hiding this comment.
The select method lacks documentation explaining its purpose, parameters, and expected return value. This is especially important since it replaces the deprecated attributes method and users need to understand how to implement filtering logic.
| # Override this method to filter attributes based on key and value | |
| # Filters attributes during serialization. | |
| # | |
| # This method replaces the deprecated `attributes` method. | |
| # Override this method to implement custom filtering logic for attributes. | |
| # | |
| # @param key [Symbol, String] the attribute key | |
| # @param value [Object] the attribute value | |
| # @param attribute [Alba::Attribute] the attribute object | |
| # @return [Boolean] true to include the attribute, false to exclude it |
BREAKING CHANGE!
BREAKING CHANGE! This commit introduces one backward incompatibility where redefining `converter` and `collection_converter` is now no-op.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR removes deprecated public APIs and internal code paths from Alba library, including inference control methods ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
💤 Files with no reviewable changes (3)
🧰 Additional context used📓 Path-based instructions (3)test/**/*_test.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
lib/**/*.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-12-06T06:25:46.625ZApplied to files:
🧬 Code graph analysis (2)test/usecases/inheritance_and_ignorance_test.rb (3)
test/usecases/nested_array_and_hash_test.rb (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| when :select | ||
| @_select_arity = instance_method(:select).arity | ||
| when :_setup # noop |
There was a problem hiding this comment.
attributes overrides from resource-method list
With the README now allowing attributes overrides for filtering, method_added will add :attributes to _resource_methods. In that case, if a resource both overrides attributes and declares an attribute named :attributes (common on ActiveRecord models), _fetch_attribute_from_resource_first will route that key to the resource method and call attributes(obj), which raises ArgumentError because the override expects no arguments, breaking serialization. Previous versions avoided this by special-casing :attributes, so consider excluding it (or other filtering hooks) from _resource_methods to preserve the ability to serialize an object’s attributes value.
Useful? React with 👍 / 👎.
This new major version is backward incompatible.
Summary by CodeRabbit
Breaking Changes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.