@gravitystorm commented on this pull request.
> + def update
@user.roles.create(:role => @role, :granter => current_user)
I would suggest that `create` is a better method name, because:
* `create` and `destroy` are more natural opposites of each other than `update`
and `destroy`
* Since the next line is also `@user.roles.create(...)` then having both the
controller-method-name and relation-method being the same (`create`) will make
it easier for developers to understand the code.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5293#pullrequestreview-2399386368
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/5293/review/2399386...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
