beam-community / bamboo

Testable, composable, and adapter based Elixir email library for devs that love piping.
https://hex.pm/packages/bamboo
MIT License
1.91k stars 345 forks source link

incorrect typespec in Adapter behavior #660

Closed epinault closed 1 month ago

epinault commented 1 year ago

So the deliver/2

https://hexdocs.pm/bamboo/Bamboo.Adapter.html#c:deliver/2

has a 2nd argument type as %{} (which is empty map.. not any map)

but if you look at

https://github.com/beam-community/bamboo/blob/master/lib/bamboo/mailer.ex#L86

you can see a config is being built

  config = build_config(opts)
  Bamboo.Mailer.deliver_later(config.adapter, email, config)

and won t be an empty map . Hammox see that an invalid contract and is correct about it. The 2nd arg should be ma() as a type

epinault commented 1 year ago

@taj @btkostner @mattpolzin @maartenvanvliet I see a lot of issue that seem unattended. is this package maintained? what the process as I see many stale MR that are old and more recent ones failing

mattpolzin commented 1 year ago

@epinault i didn’t want to leave you hanging, but I didn’t really know the answer to your question (I only currently maintain the JSONAPI package). I’d say the absence of an answer to your question is itself the answer in this case.

epinault commented 1 year ago

if I post an MR would you be able to merge and release a new version of the package?

mattpolzin commented 1 year ago

@epinault I would consider merging simple PRs but I do not have permission to publish new versions of this package to Hex. I am fairly maxed out on OSS maintenance right now so I am not in a position to seek permission to officially maintain this project.

epinault commented 1 year ago

so who has permissions? who can help with this ? I appreciate your reply by the way and totally understand the OSS maintenance burden :)

mattpolzin commented 1 year ago

It can be difficult to answer that question when the people closest in proximity to the project (beam-community, as the organization the repo is under) are not themselves the active or perhaps most recent maintainers of the project. What I believe happened is Thoughtbot decided to no longer commit to this project so it got moved from their organization over to the community organization where some existing contributors to the project carried it forward.

I'll send out an email to see if I can learn more, but no promises I will be able to reach someone with answers.

doomspork commented 1 year ago

@epinault @mattpolzin sorry we don't have things configured so this didn't tag anyone in the project and somehow slipped through my notifications.

@mattpolzin is correct. We are in the process of working through the transfer of ownership with Thoughtbot as they no longer have the capacity to maintain this and other Elixir projects. We haven't formally announced anything because we're still working through the logistics of the handoff.

Let me see about getting a note added to the README that calls out we're in the process of transferring these and figuring out the path forward with long term maintenance and feature roadmaps.

Thank you @btkostner for flagging this 🙏

epinault commented 1 year ago

any progress on this?

epinault commented 8 months ago

any chance we can merge this?

mattpolzin commented 8 months ago

@epinault when you ask if we can merge this are you referring to an open PR that I am not spotting at a glance or are you asking if we can open a new PR that fixes the issue and then merge and release that fix?

I'd suggest opening the PR with your proposed fix and then we can ping a couple folks on that PR to see who has time to help merge & release the fix.

epinault commented 8 months ago

I ll open a PR then! and link it here! thanks!

epinault commented 8 months ago

just opened the MR

github-actions[bot] commented 1 month ago

This issue has been automatically marked as "stale:discard". We are sorry that we haven't been able to prioritize it yet. If this issue still relevant, please leave any comment if you have any new additional information that helps to solve this issue. We encourage you to create a pull request, if you can. We are happy to help you with that.

btkostner commented 1 month ago

PR #662 has been merged so I'll close this issue. Looks like most of the configuration has been updated for releases, so I imagine @doomspork will be making a new one soon.