feat: add swoosh module #149
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
zachallaun
reviewed
Nov 12, 2024
lib/igniter/libs/swoosh.ex
Outdated
| @doc "Moves to the use statement in a module that matches `use Swoosh.Mailer`" | ||
| @spec move_to_mailer_use(Igniter.t(), Sourceror.Zipper.t()) :: | ||
| :error | {:ok, Sourceror.Zipper.t()} | ||
| def move_to_mailer_use(_igniter, zipper) do |
There was a problem hiding this comment.
I have mixed feelings on requiring an unused argument here.
It's consistent in that all the functions in Libs.Phoenix and Libs.Ecto require it, but all those functions actually use the argument. Maybe this function starts using it in the future in order to be more intelligent or efficient, but I somewhat doubt it.
We should probably either:
- Remove the argument and make it
move_to_mailer_use/1, or - Change the signature to
move_to_mailer_use(%Igniter{}, zipper)to at least ensure the right thing is being passed in, even if it's not being used.
I think 1. is preferable (we can always deprecate it later), but I could be convinced of 2.
There was a problem hiding this comment.
I'm fine with 1. I just copied over some of the code from the phoenix module last week and made it work.
I should have taken another look at it today before pushing.
Co-authored-by: Zach Allaun <[email protected]>
zachallaun
approved these changes
Nov 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
needed for team-alembic/ash_authentication#835