-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
Support component module prefix #146
Conversation
bluzky
commented
Feb 5, 2025
•
edited
Loading
edited
- Support module prefix
- Fix import when install sidebar
Reviewer's Guide by SourceryThis pull request introduces support for a configurable component module prefix. It modifies the Class diagram for updated module structureclassDiagram
class ModuleStructure {
+get_module_prefix()
}
note for ModuleStructure "Changed from fixed 'Web' suffix to configurable prefix or 'Web.Components'"
class ComponentGenerator {
+insert_target_module_name(source, file_name)
+maybe_apply_additional_insertions(source, module_name, file_name)
}
note for ComponentGenerator "Updated to use new module prefix pattern"
ModuleStructure --> ComponentGenerator: uses
Flow diagram for module prefix resolutiongraph TD
A[Start] --> B{Check :component_module_prefix}
B -->|Not configured| C[Build from app name]
C --> D[Camelize app name]
D --> E[Append 'Web.Components']
B -->|Configured| F[Use configured prefix]
E --> G[Return prefix]
F --> G
G --> H[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @bluzky - I've reviewed your changes - here's some feedback:
Overall Comments:
- The conversion of the module prefix for the 'use SaladUI, :component' replacement uses String.trim_trailing(module_prefix, "s"), which might be brittle if the naming convention changes. Consider abstracting this logic into a helper function or making the transformation more explicit to better document the intended behavior.
- The new regex replace for 'import SaladUI.' now transforms it to 'import #{module_prefix}.'. Make sure this generic replacement doesn't unintentionally match and replace strings that shouldn't change. A more specific regex might help avoid unexpected behavior if there are similarly named modules.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 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.