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

Use Layout/BeginEndAlignment style start_of_line #258

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

dylanahsmith
Copy link
Contributor

Problem

Currently we disable the Layout/RescueEnsureAlignment in Shopify Core and have a TODO to re-enable it. There used to be a comment saying to renable it when rubocop/rubocop#6433 was fixed but it didn't get renabled after that, I assume because it doesn't have the desired effect.

For instance, it autocorrects with changes like this

       host = begin
         URI.parse(value).host
-      rescue URI::Error
-        nil
+             rescue URI::Error
+               nil
       end

which is pretty close to the original issue reported in that issue, with the difference being the presence of the begin keyword.

Solution

Using the Layout/BeginEndAlignment style of start_of_line avoids the above problem and uses a style that seems consistent with our existing coding style. This style is used by the Layout/RescueEnsureAlignment cop when it is enabled.

If this is too disruptive a change to introduce without a major release, the alternative would be to disable the Layout/RescueEnsureAlignment cop since it doesn't have the desired behaviour without Layout/BeginEndAlignment also enabled.

@dylanahsmith
Copy link
Contributor Author

cc @Edouard-chin since you have context on rubocop/rubocop#6433

Copy link
Contributor

@volmer volmer left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Could you please include this rule in the README as well?

rubocop.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@Edouard-chin Edouard-chin left a comment

Choose a reason for hiding this comment

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

👍

@dylanahsmith dylanahsmith merged commit 364c3c3 into main Apr 20, 2021
@dylanahsmith dylanahsmith deleted the begin-end-align-start-of-line branch April 20, 2021 15:40
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems April 23, 2021 12:56 Inactive
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.

None yet

4 participants