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

Change define_helpers parameters to String #5492

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gardnerapp
Copy link

The define_helpers method takes an instance of Devise::Mapping as an argument, which is a very large object. Within the method a local variable mapping, which is set to the name instance variable of the mapping object. This variable is then used in the method to meta program helper methods like user_signed_in? etc.

It is unnecessary to pass an entire Devise::Mapping to define_helpers. Instead I propose passing the @name of the mapping directly to define_helpers. The assignment of the mapping variable can now also be removed. This change simplifies code and speeds up the program by avoiding passing large objects to functions.

@p8
Copy link
Contributor

p8 commented May 31, 2022

Hey @gardnerapp, thanks for contributing to Devise!
It seems like passing the mapping instead of the name was changed in:
ef841ca
It doesn't seem to be a requirement for the change, just a refactoring, and the method is marked as :nodoc: so I'm guessing it's ok to change it.

@gardnerapp
Copy link
Author

I'm new to open source, what does :nodoc mean?

@p8
Copy link
Contributor

p8 commented Jun 2, 2022

@gardnerapp Sorry, :nodoc: means it's not documented as public API.
Third parties shouldn't use it because it might change in the future, like the changes in this PR.

:nodoc: / :nodoc: all
Don't include this element in the documentation. For classes and modules, the methods, aliases, constants, and attributes directly within the affected class or module will also be omitted. By default, though, modules and classes within that class of module will be documented. This is turned off by adding the all modifier.

https://ruby-doc.org/stdlib-1.9.1/libdoc/rdoc/rdoc/index.html#label-Directives

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

Successfully merging this pull request may close these issues.

2 participants