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

incorrect typespec in Adapter behavior #660

Closed
epinault opened this issue Oct 6, 2023 · 14 comments
Closed

incorrect typespec in Adapter behavior #660

epinault opened this issue Oct 6, 2023 · 14 comments

Comments

@epinault
Copy link
Contributor

epinault commented Oct 6, 2023

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
Copy link
Contributor Author

epinault commented Oct 6, 2023

@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
Copy link
Member

@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
Copy link
Contributor Author

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

@mattpolzin
Copy link
Member

@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
Copy link
Contributor Author

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

@mattpolzin
Copy link
Member

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
Copy link
Member

@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
Copy link
Contributor Author

any progress on this?

@epinault
Copy link
Contributor Author

epinault commented Mar 7, 2024

any chance we can merge this?

@mattpolzin
Copy link
Member

@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
Copy link
Contributor Author

epinault commented Mar 7, 2024

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

@epinault
Copy link
Contributor Author

epinault commented Mar 7, 2024

just opened the MR

Copy link
Contributor

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants