feat(Localizer): add GetStringFromBaseType logic support base type#6006
feat(Localizer): add GetStringFromBaseType logic support base type#6006
Conversation
|
Thanks for your PR, @kimdiego2098. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideThis pull request updates the File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @kimdiego2098 - I've reviewed your changes - here's some feedback:
- Consider replacing
Assembly.GetType(typeName)withType.GetType(typeName)for more reliable type resolution across assemblies. - When a base class resource is found, assign a new
LocalizedStringinstance toret, not just the string value, to maintain localization metadata. - The condition for calling
HandleMissingResourceItemafter the base class lookup should be based on whether the lookup succeeded, rather than checking ifretis null.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6006 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 670 670
Lines 30620 30645 +25
Branches 4355 4361 +6
=========================================
+ Hits 30620 30645 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Diego2098 <82756760+kimdiego2098@users.noreply.github.com>
Link issues
fixes #6007
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enhancements: