search

elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.85k stars 586 forks source link

Replace uses of plug_multipart.erl with Elixir version? #1206

Closed HansGlimmerfors closed 8 months ago

HansGlimmerfors commented 8 months ago

(I'm not sure whether I should open an issue for this since there is neither a bug to report, nor a "feature" request?)

I noticed that plug currently only has one .erl-file in the project which - based on the comments and copyright notices - seems to come from cowboy. Is there a reason to keep this module as a lone Erlang module rather than rewriting it to an Elixir module?

Here's a proposal for deprecating the exported functions from plug_multipart and replacing them with equivalent functions in Plug.Conn.Multipart. :)

josevalim commented 8 months ago

Hi @HansGlimmerfors! The API in the Erlang file is private, so no one should rely on it. It was copied from Cowboy because they had the original multipart functionality already implemented.

What is your use case? I am not very inclined to add this functionality as part of our public API.

HansGlimmerfors commented 8 months ago

I don't have a use case for needing this, and I'm not especially pushing for the change. I was just looking at the implementation out of curiosity, and could find no obvious reason for preferring the Erlang code over Elixir code (like the rest of the project). Since it's not an overly large file and there's only one, it felt like something that could be rewritten.

But that was given that the code should be used... I had failed to notice that the doc tags in plug_multipart were actually commented out, and there is no documentation referring to the module. If the goal is to keep the API private, then it might be better to keep the Erlang code. :relaxed: