Skip to content

Rubocop multiline method calls#4919

Merged
mitchellhenke merged 2 commits intomainfrom
rubocop-multiline-method-calls
Apr 16, 2021
Merged

Rubocop multiline method calls#4919
mitchellhenke merged 2 commits intomainfrom
rubocop-multiline-method-calls

Conversation

@mitchellhenke
Copy link
Contributor

Based on the conversation here: #4876 (comment)

Goal is to require this:

      @register_user_email_form = RegisterUserEmailForm.new(
        recaptcha_results: validate_recaptcha,
        analytics: analytics,
      )

Instead of allowing this:

      user, result = RequestPasswordReset.new(email: email,
                                              request_id: request_id,
                                              analytics: analytics).perform

The rule changes required to get that resulted in some other alignment and spacing fixes

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM big fan

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Nice 👍

Comment on lines 28 to 31
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, would the following be an error?

Suggested change
params.require(:resolution_result).permit(
:exception, :success, :timed_out, :transaction_id,
errors: {}, context: {}
)
params.require(:resolution_result).permit(
:exception,
:success,
:timed_out,
:transaction_id,
errors: {},
context: {},
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't take any issues with it as-is. I would maybe consider splitting it up into an array that gets passed into the function (if it accepts it in that form)

permitted_params = [:exception, :success, :timed_out, :transaction_id, errors: {}, context: {}]
params.require(:resolution_result).permit(permitted_params)

@mitchellhenke mitchellhenke force-pushed the rubocop-multiline-method-calls branch from 7391563 to 1fc58b6 Compare April 16, 2021 13:27
@mitchellhenke mitchellhenke merged commit f0c5fe8 into main Apr 16, 2021
@mitchellhenke mitchellhenke deleted the rubocop-multiline-method-calls branch April 16, 2021 13:43
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.

3 participants