aws-beam / aws-codegen

Code generator for AWS clients in Erlang and Elixir.
Other
53 stars 49 forks source link

Application name conflict between Erlang and Elixir implementations #73

Open hauleth opened 3 years ago

hauleth commented 3 years ago

Right now both of the implementations use aws as an application name, which may conflict if there will be need to include both of them in a single project (for example as a transitive dependency).

Possible solutions:

hauleth commented 3 years ago

Elixir application could be defined as something like:

modules = Application.spec(:aws, :modules)

for module <- modules do
  name =
    module
    |> Atom.to_string()
    |> String.slice(4..-1)
    |> String.split("_")
    |> Enum.map(&String.capitalize/1)

  # Here we probably should replace some parts with all uppercase names,
  # for example `Ec2` should probably be available as `EC2`.
  module_name = Module.concat(["AWS" | name])

  defmodule module_name do
    # Here we just delegate all exported functions from `module` in new module
    for {func_name, arity} <- module.module_info(:exports),
      func_name not in ~w[module_info]a
    do
      # With EEP 48 we probably could make arguments name more clear than
      # generated gibberish
      args = Macro.generate_arguments(arity, __MODULE__)

      # We lose the documentation there a little, but with EEP 48 we probably would be able
      # to retain all documentation as-is
      defdelegate unquote(func_name)(unquote_splicing(args)), to: module
    end
  end
end

Alternative is to generate modules that will just pass calls to the Erlang modules in aws_codegen so we could have clearer documentation and codebase.

philss commented 3 years ago

Hey @hauleth :wave: Thanks for the suggestions!

I think for now we should go with the first option, which is to rename the applications to the name of the project. In my mind, the problem of merging them is that we will have double the modules, thus increasing the compilation time. But the idea worth exploring, since @robertoaloi also mention this in https://github.com/aws-beam/aws-elixir/pull/65#issuecomment-808695831

jkakar commented 3 years ago

One of my motivations for creating separate libraries was that I wanted it to be easy for people to jump into the code and read it, to find out what it's doing. That's also why the code generator tries to generate code that is pretty to humans (though passing that code through a code formatter like erlfmt or mix format is a better choice now over trying to make the code generator care about formatting). I also wanted to be able to evolve the APIs differently, to be able to produce the best APIs possible for each supported language. I'm sharing this as historic context, more than anything.

philss commented 3 years ago

I'm afraid we have to revert the commit that renames aws-elixir application name because it would require us to publish a new Hex package and retire the current one.

I don't think this is necessary. Instead, we could just rename the application name of our aws-erlang package that has already the correct package name. The Elixir package would keep the aws package and application name.

@jkakar @robertoaloi what do you think?

hauleth commented 3 years ago

@philss I think that the ultimate goal would be to merge these two projects into hybrid like opentelemetry_api where one project can be used by both Rebar3 and Mix projects. This should be IMHO the best approach, as instead of generating and updating 2 separate projects on each change, everything could be done within single update.

robertoaloi commented 3 years ago

I don't have a problem with the aws_erlang name, since that would avoid the inconsistency between the application and package name. The opentelemetry_api approach seems interesting and something we should look into, though.

onno-vos-dev commented 6 months ago

Spring cleaning is good they say :-)

I discussed this briefly with a colleague and also with @hauleth on Slack who re-raised this issue.

I'll take a look at opentelemetry_api over the coming weeks or so and see how we could use this. I'll update this issue and ping the necessary people when we get to that stage.

I'm guessing it would have to mean a new package called aws_api that would house both packages in that case? (I'm not very familiar with opentelemetry_api so hence the question/investigation that would need doing 👍)