-
Notifications
You must be signed in to change notification settings - Fork 62
fix: multi-aura polarity support #828
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
Conversation
WalkthroughThe pull request extends polarity support to handle both single values and arrays. The transformer logic is updated to recursively process polarity arrays, type definitions expand to permit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
data/json/All.json (1)
966850-966850: Verify that the itemCount change is intentional.Similar to the Moonlight Dragonlily change, this itemCount modification for Sunlight Dragonlily appears unrelated to the multi-aura polarity support objective. Please confirm this is an intentional data correction and verify the commit structure.
🧹 Nitpick comments (2)
build/wikia/transformers/transformPolarity.mjs (2)
7-9: Array handling logic looks solid.The recursive approach correctly transforms arrays of polarity values, enabling multi-aura polarity support as intended.
Optional: Simplify redundant type check
The
typeof field !== 'string'check is redundant sinceArray.isArray()never returns true for strings. You can simplify:- if (typeof field !== 'string' && Array.isArray(field)) { + if (Array.isArray(field)) { return field.map((f) => transform(f)); }
21-29: Update JSDoc to reflect array support.The JSDoc describes
AuraPolarity,StancePolarity, andPolarityas strings, but the code now handles arrays for these fields (lines 7-9). Consider updating the documentation to reflect that these parameters acceptstring | string[].Suggested JSDoc update
/** * Transform polarity - * @param {string} [AuraPolarity] string designating the polarity for the aura compatibility of a Warframe + * @param {string|string[]} [AuraPolarity] string or array of strings designating the polarity for the aura compatibility of a Warframe * @param {module:warframe-items.Item} target item for which to standardize polarities - * @param {string} [StancePolarity] string designating the polarity for the stance compatibility of a melee weapon - * @param {string} [Polarity] string designating the polarity for the aura compatibility of a mod slot or Mod + * @param {string|string[]} [StancePolarity] string or array of strings designating the polarity for the stance compatibility of a melee weapon + * @param {string|string[]} [Polarity] string or array of strings designating the polarity for the aura compatibility of a mod slot or Mod * @param {Array<string>} [Polarities] list of strings designating polarities on a weapon or warframe * @returns {*} */
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
build/wikia/transformers/transformPolarity.mjs(1 hunks)data/json/All.json(3 hunks)index.d.ts(1 hunks)test/lookup.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-28T01:41:19.914Z
Learnt from: SlayerOrnstein
Repo: WFCD/warframe-items PR: 778
File: build/parser.mjs:568-574
Timestamp: 2025-11-28T01:41:19.914Z
Learning: In the WFCD/warframe-items repository, for Nightwave Act processing, the regex difference between `build/parser.mjs` (using `/[0-9]{1,3}$/` with anchor) and `build/build.mjs` (using `/[0-9]{1,3}/` without anchor) is intentional and serves different purposes in each file.
Applied to files:
test/lookup.mjs
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (4)
index.d.ts (1)
354-354: Type change correctly supports multi-aura polarity.Jade is the first Warframe to possess two mod aura slots, making the union type
Polarity | Polarity[]necessary and appropriate. This change maintains type safety while supporting both single aura values (backward compatibility) and arrays of polarities (new feature). The type correctly enables the PR objective of multi-aura polarity support.test/lookup.mjs (1)
10-10: File is newly created, not a modification of an existing endpoint.The
test/lookup.mjsscript was introduced with the/warframes/${lookup}endpoint in its initial implementation. There is no prior version showing a change from any other endpoint. The API endpoint is functional and suitable for the warframe-specific lookup functionality.Likely an incorrect or invalid review comment.
data/json/All.json (1)
416748-416748: LGTM! Multi-aura array format implemented correctly.The change from a comma-separated string to an array format correctly enables multi-aura polarity support as intended by this PR.
build/wikia/transformers/transformPolarity.mjs (1)
15-15: Improved error logging.The enhanced error message with field context and error details will aid debugging.
|
🎉 This PR is included in version 1.1272.85 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What did you fix?
aura stuff
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.