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

Scope modules created by defparams to defining module namespace #24

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

jgautsch
Copy link
Contributor

@jgautsch jgautsch commented Oct 8, 2017

I ran into an issue with calling defparams with the same param schema name in different controllers. This PR makes doing so possible, which I think is convenient because needing to fully type out a unique name for every parameter schema seems needlessly restrictive (especially in a large app with many resources/controllers).

This PR allows for the following usage, which previously wouldn't compile, because both controllers would try to create a Params.CreateParams module:

defmodule MyApp.UserController do
  ...
  defparams create_params %{first_name: :string} # It's convenient to match param schema names with controller action names
  
  def create(conn, params), do: text(conn, "created")
end

defmodule MyApp.OrganizationController do
  ...
  defparams create_params %{employees: :integer}

  def create(conn, params), do: text(conn, "created")
end

Now the defparams line in each controller will define Params.MyApp.UserController.CreateParams and Params.MyApp.OrganizationController.CreateParams modules, respectively.

Feedback appreciated, thanks for the great project.

@jgautsch
Copy link
Contributor Author

jgautsch commented Oct 8, 2017

CI seems to be failing due to a couple of failed wgets rather than the tests (I don't see a way to re-run)

@take-five
Copy link
Collaborator

@vic if you don't have objections, I'll merge this in 24 hours.

@take-five take-five self-assigned this Oct 9, 2017
@take-five
Copy link
Collaborator

Note: this change will entail in minor version increase

@jgautsch
Copy link
Contributor Author

@vic @take-five If there are no objections to address would you mind merging and cutting a release when you have a chance? Thanks again.

@take-five take-five merged commit a1efe0c into vic:master Oct 12, 2017
@take-five
Copy link
Collaborator

@jgautsch Version 2.1.0 is released

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.

2 participants