Skip to content

rewrite_python_shebang: move loop invariants out of loop#7128

Merged
iMichka merged 1 commit intoHomebrew:masterfrom
bayandin:patch-1
Mar 6, 2020
Merged

rewrite_python_shebang: move loop invariants out of loop#7128
iMichka merged 1 commit intoHomebrew:masterfrom
bayandin:patch-1

Conversation

@bayandin
Copy link
Copy Markdown
Contributor

@bayandin bayandin commented Mar 5, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

regex and maximum_regex_length variables do not change inside Pathname(".").find do |f| loop, so move them out of the loop.

brew style complains:

Library/Homebrew/language/python.rb:92:30: C: Do not compute the size of statically sized objects.
      maximum_regex_length = "#! /usr/bin/env pythonx.yyy$".length

Do I need to fix it something like this

-      maximum_regex_length = "#! /usr/bin/env pythonx.yyy$".length
+      maximum_regex_length = 28 # the lenght of "#! /usr/bin/env pythonx.yyy$"

Or add # rubocop:disable Performance/FixedSize
Or something else?

@iMichka
Copy link
Copy Markdown
Member

iMichka commented Mar 5, 2020

Both are fine. I would go for maximum_regex_length = 28 # the lenght of "#! /usr/bin/env pythonx.yyy$" because I prefer not to carry on lint disabling lines around in my code.

@bayandin
Copy link
Copy Markdown
Contributor Author

bayandin commented Mar 5, 2020

Thanks @iMichka, fixed now

@Bo98
Copy link
Copy Markdown
Member

Bo98 commented Mar 5, 2020

Tempting to not point out the series of Cmd+C and Cmd+V which has led the misspelling "lenght" to travel through the original post, into the comments and then the code.

@bayandin
Copy link
Copy Markdown
Contributor Author

bayandin commented Mar 5, 2020

Tempting to not point out the series of Cmd+C and Cmd+V which has led the misspelling "lenght" to travel through the original post, into the comments and then the code.

And the lack of Cmd+C and Cmd+V when I wrote it initially 😄

@iMichka iMichka merged commit d345916 into Homebrew:master Mar 6, 2020
@bayandin bayandin deleted the patch-1 branch March 6, 2020 08:47
@lock lock bot added the outdated PR was locked due to age label Apr 15, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants