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

Add request! function #219

Merged
merged 5 commits into from
Apr 11, 2023
Merged

Add request! function #219

merged 5 commits into from
Apr 11, 2023

Conversation

jwien001
Copy link
Contributor

There are some cases where I want to raise an exception on request failure. Instead of writing the with block every time or writing a helper function in my own code, it would be convenient to have a bang variation of request.

Let me know what you think. If this is accepted, it would be great if you could also cut a release after merging so I could start using the new function. Thanks!

test/finch_test.exs Outdated Show resolved Hide resolved
@sneako
Copy link
Owner

sneako commented Mar 30, 2023

Hi and thanks for the PR! I'm willing to add this, please just update the tests as pointed out by @dvic and move the new function definition to get rid of this warning:

clauses with the same name and arity (number of arguments) should be grouped together, "def request/3" was previously defined (lib/finch.ex:312)

@jwien001
Copy link
Contributor Author

jwien001 commented Apr 3, 2023

I didn't know about Credo, but I've fixed the error and now Credo and tests are all passing locally.

Ubuntu 18.04 is no longer supported
@jwien001
Copy link
Contributor Author

jwien001 commented Apr 5, 2023

It looks like CI was failing because Github deprecated and removed the Ubuntu 18.04 Actions runner image: actions/runner-images#7388. I've updated the runner finch uses to 20.04, which should unblock CI.

@jwien001
Copy link
Contributor Author

Hello, it looks like CI is passing. Do you have any more feedback or anything else I should do to prepare this PR for merging?

@sneako sneako merged commit a345a78 into sneako:main Apr 11, 2023
@jwien001
Copy link
Contributor Author

Hey @sneako, thanks for merging this in! Would you mind cutting a new release too?

@sneako
Copy link
Owner

sneako commented Apr 13, 2023

just published v0.16.0 🎉

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.

4 participants