Skip to content
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

refactor:test(ex/conn_case): split conn creation into helper function #2864

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

firestack
Copy link
Member

I was working on upgrading some of Skate to latest Phoenix for a personal project, and ended up refactoring ConnCase to see if I could make it any cleaner. I liked this, but it's not necessary, let me know what you think!

@firestack firestack force-pushed the kf/update/phoenix/conn-case-fn branch from b487760 to 714708f Compare October 21, 2024 14:35
@firestack firestack marked this pull request as ready for review October 21, 2024 15:04
@firestack firestack requested a review from a team as a code owner October 21, 2024 15:04
@firestack firestack linked an issue Oct 21, 2024 that may be closed by this pull request
@firestack firestack force-pushed the kf/update/phoenix/conn-case-fn branch from 714708f to aedba9a Compare October 21, 2024 15:23
@firestack firestack force-pushed the kf/update/phoenix/conn-case-fn branch 2 times, most recently from 9dbd5d3 to dcbd7ac Compare October 23, 2024 13:53
@firestack firestack force-pushed the kf/update/phoenix/conn-case-fn branch from dcbd7ac to 86109ea Compare October 25, 2024 15:02
Copy link
Collaborator

@hannahpurcell hannahpurcell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I find this easier to read than previously, but I'm not opposed to it if you find it easier to use.

For me, I mildly prefer the single user / resource creation so that it didn't need to be a reusable function.

However, one thing I don't understand about the old file, and which you fix in your changes, is the second User.upsert(username, email) that happens for each case in the cond do. That does seem unneeded given the user = User.upsert(username, email) that happens previously.

@firestack
Copy link
Member Author

Rethinking this based on the functions inserted into ConnCase by mix phx.gen.auth.

@firestack firestack marked this pull request as draft October 31, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to New Phoenix Template
2 participants