-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Modify salad.init task to automatically add TwMerge.Cache #107
Conversation
Reviewer's Guide by SourceryThis PR enhances the Sequence diagram for salad.init task modificationsequenceDiagram
participant User
participant MixTask as Mix.Tasks.Salad.Init
participant Patcher as SaladUI.Patcher
participant ElixirPatcher as SaladUI.Patcher.ElixirPatcher
User->>MixTask: Run salad.init
MixTask->>MixTask: Determine environment and paths
MixTask->>MixTask: Create component path
MixTask->>MixTask: Write config
MixTask->>MixTask: Initialize TwMerge.Cache
MixTask->>ElixirPatcher: Patch application.ex
ElixirPatcher->>ElixirPatcher: Read application.ex
ElixirPatcher->>ElixirPatcher: Update content with TwMerge.Cache
ElixirPatcher->>MixTask: Return success or error
MixTask->>MixTask: Patch CSS, JS, Tailwind config
MixTask->>MixTask: Copy Zag files
MixTask->>MixTask: Install node dependencies
MixTask->>User: Display completion message
Class diagram for new ElixirPatcher moduleclassDiagram
class SaladUI.Patcher {
+patch_elixir_application(application_file_path, new_children, description)
}
class SaladUI.Patcher.ElixirPatcher {
+patch_application_supervisor(application_file_path, new_children, description)
-update_content(content, new_children, description)
-single_line_list?(list_content)
-extract_indentation(list_content)
-build_addition(new_children, description, element_indentation)
}
SaladUI.Patcher --> SaladUI.Patcher.ElixirPatcher : uses
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 @selenil - I've reviewed your changes - here's some feedback:
Overall Comments:
- The PR includes additional changes (zag files) beyond what's described in the title/description. Please either update the description to reflect all changes or split this into separate PRs for better clarity.
- The new ElixirPatcher module would benefit from test coverage to ensure reliable supervisor modifications across different application configurations.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 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.
lib/mix/tasks/salad.init.ex
Outdated
@@ -155,6 +175,25 @@ defmodule Mix.Tasks.Salad.Init do | |||
:ok | |||
end | |||
|
|||
defp copy_zag_files(assets_path) do |
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.
suggestion: Consider adding explicit error handling for file operations
File operations could fail for various reasons (permissions, disk space). Consider wrapping operations in a with statement to handle errors gracefully
defp copy_zag_files(assets_path) do
with {:ok, _} <- ensure_writable(assets_path),
:ok <- Mix.shell().info("Copying zag files to assets folder") do
source_path = Path.join(assets_path, "zag")
f9e113e
to
43cfd8a
Compare
Thanks @selenil, would you mind adding unit test for new patcher. |
The
salad.init
task now also patches theapplication.ex
file to automatically includeTwMerge.Cache
as part of the setup process.Summary by Sourcery
Modify the salad.init task to automatically include TwMerge.Cache in the application supervisor setup. Add functionality to patch the Elixir application supervisor with new children, enhancing the initialization process.
New Features:
Enhancements: