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

Support autocorrect for Lint/Loop cop #8383

Merged
merged 1 commit into from Aug 5, 2020

Conversation

fatkodima
Copy link
Contributor

No description provided.

@marcandre
Copy link
Contributor

marcandre commented Jul 22, 2020

I think you might be loosing comments in this case:

begin
  something
end while # don't remove me
   test

May not be worth the worry.

@fatkodima
Copy link
Contributor Author

fatkodima commented Jul 22, 2020

I hope it is impossible to find the code like that 😄

This is much more possible:

begin
  something
end while test # don't remove me

And it will be correctly handled.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 23, 2020

I wonder why we made this a lint cop, as I definitely don't remember it today. Seems we could have written better documentation. 😆

@marcandre
Copy link
Contributor

Am I weird?

I prefer

loop do
  # ...
  break if condition
end

But I'd rather have

begin
   # ...
end while condition
# than
loop do
   # ...

  break unless condition
end

The answer is probably "yeah". I'm glad it's rare that there's a reason to loop

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 23, 2020

You're weird for sure, but then again - I've never had to use loop in my entire Ruby career. 😆

@fatkodima
Copy link
Contributor Author

I've never had to use loop in my entire Ruby career. 😆

I have used loop, but I would say I have never used for loop (but I do not see it that bad as everybody talks), do not remember if I ever really used protected keyword with its intended purpose and never found the case where I really needed to made the choice between proc and lambda because of their differences.

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 5, 2020

The changes look good, but your branch has to be rebased on top of the current master branch due to merge conflicts.

@fatkodima
Copy link
Contributor Author

Rebased.

@bbatsov bbatsov merged commit 426814e into rubocop:master Aug 5, 2020
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

3 participants