Skip to content

Fixes violations of and reenables rubocop Lint/EndAlignment.#9140

Merged
ashercodeorg merged 2 commits into
stagingfrom
rubocopLintEndAlignment
Jun 28, 2016
Merged

Fixes violations of and reenables rubocop Lint/EndAlignment.#9140
ashercodeorg merged 2 commits into
stagingfrom
rubocopLintEndAlignment

Conversation

@ashercodeorg

Copy link
Copy Markdown
Contributor

Fixes generated manually. Note also that our code seems to surface a bug in rubocop, as rubocop fails to find the second failure in each of these file.

Might mess around with this, trying to produce a simple example with this failure (trivial examples do not surface the bug), reporting an issue if I am able to do so.

@ashercodeorg

Copy link
Copy Markdown
Contributor Author

Ping.

@mehalshah

Copy link
Copy Markdown
Contributor

Sorry - looking now

@mehalshah

Copy link
Copy Markdown
Contributor

LGTM

'cookie.filter_except("NO_CACHE");'
else
cookies.map{ |c| extract_cookie(c)}.join + "cookie.filter_except(\"#{cookies.join(',')}\");"
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For Style/AlignParameters we've preferred fixed indentation. I wonder if we should use parens here keep the case statement at a normal indent?

out << (
  case cookies
  when 'all'
    '# Allow all request cookies.'
  when 'none'
    'cookie.filter_except("NO_CACHE");'
  else
    cookies.map{ |c| extract_cookie(c)}.join + "cookie.filter_except(\"#{cookies.join(',')}\");"
  end
)

We've been using this pattern a lot in React (example).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done (also removed unnecessary assignment of out to empty string).

@ashercodeorg ashercodeorg merged commit 7e897b4 into staging Jun 28, 2016
@ashercodeorg ashercodeorg deleted the rubocopLintEndAlignment branch June 28, 2016 00:31
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.

3 participants