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

Actionpack typo fixes. #35030

Merged
merged 1 commit into from
Jan 28, 2019
Merged

Actionpack typo fixes. #35030

merged 1 commit into from
Jan 28, 2019

Conversation

alkesh26
Copy link
Contributor

@alkesh26 alkesh26 commented Jan 23, 2019

No description provided.

@rails-bot rails-bot bot added the actionpack label Jan 23, 2019
@@ -7,7 +7,11 @@

module AbstractController
class DoubleRenderError < Error
DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like \"redirect_to(...) and return\"."
DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. " \
Copy link
Member

Choose a reason for hiding this comment

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

For breaking up strings, you could also use a heredoc and squish.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are cosmetic changes which we don’t accept.

Copy link
Member

Choose a reason for hiding this comment

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

Yup. Can you please revert. If you want to fix typos, it is fine. If you want to change the way the code looks, please don't.

@@ -7,7 +7,11 @@

module AbstractController
class DoubleRenderError < Error
DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like \"redirect_to(...) and return\"."
DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. " \
Copy link
Contributor

Choose a reason for hiding this comment

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

These are cosmetic changes which we don’t accept.

@@ -73,7 +73,7 @@ def initialize(controller, env, defaults)
# The primary options are:
# * <tt>:partial</tt> - See <tt>ActionView::PartialRenderer</tt> for details.
# * <tt>:file</tt> - Renders an explicit template file. Add <tt>:locals</tt> to pass in, if so desired.
# It shouldnt be used directly with unsanitized user input due to lack of validation.
# It shouldn't be used directly with unsanitized user input due to lack of validation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really necessary?

@@ -795,7 +795,7 @@ def init_with(coder) # :nodoc:
@permitted = coder.map["ivars"][:@permitted]
when "!ruby/object:ActionController::Parameters"
# YAML's Object format. Only needed because of the format
# backwardscompability above, otherwise equivalent to YAML's initialization.
# backwards compatibility above, otherwise equivalent to YAML's initialization.
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d rather keep the git history here.

Copy link
Member

Choose a reason for hiding this comment

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

I think it is fine here. That word doesn't exist 😄

@alkesh26 alkesh26 changed the title Actionpack typo fixes and breaking long string into multiple lines. Actionpack typo fixes. Jan 28, 2019
@alkesh26
Copy link
Contributor Author

@kaspth @rafaelfranca I think this is good to go now. We can merge this.

@rafaelfranca rafaelfranca merged commit 2f9f699 into rails:master Jan 28, 2019
@alkesh26 alkesh26 deleted the actionpack-long-string-indentation-and-typo-fix branch January 29, 2019 04:25
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

4 participants