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

Accept error tuples in emit #13

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Accept error tuples in emit #13

wants to merge 2 commits into from

Conversation

benwilson512
Copy link
Member

No description provided.

@benwilson512
Copy link
Member Author

Thoughts @LostKobrakai ?

@LostKobrakai
Copy link
Contributor

I generally like it. The typespec would need updating though.

Also I'd skip the arity-1 callback option. If we introduce the new version of ecto Repo.transaction does pass the repo to both multi and functions passed in. So I'd go for fn aggregate, repo -> or fn aggregate, repo, changes ->. I feel this is nearer to ecto, where at least for multi usage repo is also passed all the time.

@benwilson512
Copy link
Member Author

benwilson512 commented Jan 28, 2020

That's fair, can you elaborate on use of changes? I'm not sure I see when it would be used.

@LostKobrakai
Copy link
Contributor

It‘s the changes provided by Ecto.Multi.run and ….merge.

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.

2 participants