Skip to content

Conversation

@florius0
Copy link
Contributor

@florius0 florius0 commented Apr 16, 2022

Summary

This PR fixes Witchcraft.Chain.do_notation/2.
Previously it ignored second argument assuming any chainer is Witchcraft.Chain.chain/2.

However, there is a call of do_notation/2 across Witchcraft codebase in which chainer other then chain/2 is passed:

defmacro async(sample, do: input) do
returnized = desugar_return(input, sample)
Witchcraft.Chain.do_notation(returnized, &Witchcraft.Monad.async_bind/2)
end

Test plan (required)

mix test should be enough

Closing issues

Fixes #108

After Merge

  • Does this change invalidate any docs or tutorials? If so ensure the changes needed are either made or recorded
  • Does this change require a release to be made? Is so please create and deploy the release

@florius0
Copy link
Contributor Author

Moreover, the fact mention above can be proved as follows.

Running this snippet before would've returned [links: []], which points that no process were spawn_link'ed to shell process (if the snippet is run from shell).

async [] do
  let a = 1
  let b = 2
  
  return a + b
  
  return Process.info(self, :links)
end

Now, it shows that the process is indeed linked: [links: [#PID<0.238.0>]]

@florius0
Copy link
Contributor Author

@QuinnWilton could you take a look?

@cognivore
Copy link

cognivore commented Mar 3, 2023

Wow, thanks for the PR, @florius0 I'll review it shortly and incorporate into doma-engineering fork and provide feedback here + at fission discord. 💙 💛

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.

Witchcraft.Chain.do_notation/2 intentionally ignores chainer

2 participants