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

Adding support for rooms #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding support for rooms #30

wants to merge 1 commit into from

Conversation

copenhas
Copy link
Member

@copenhas copenhas commented Mar 8, 2016

Trying to accomplish #26

end

def create_changeset(params) do
%Room{ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have questioned whether or not we should do this over the convention that I see where *changeset functions take in the model.

The usual way this is defined is like this:

def create_changeset(model, params) do
  model
  |> ...
end

that idiom is every I have seen, the default changeset for example is defined with the same parameters. If I had to guess it's for pipelining.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should follow the conventions then! I dropped the model parameter because it was a create and I was thinking they shouldn't provide one.

Hey do you remember why we went with a create_changeset? Could most things be handled by a regular changeset function? I know we discussed this and something came up that led to using multiple changeset functions. I'm thinking for Room I might switch this to a single changeset since right now there are no differences in context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah we used create_changeset in the User module and that was because it was a bit of an odd case where the password hash was being generated which would not want not want when updating a user etc.

In your case I think changeset would be fine. I think I have used changeset in all the others like the Character module and any of the newer ones added.

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

Successfully merging this pull request may close these issues.

None yet

2 participants