Skip to content

Manual Rubocop Fixes#361

Merged
mattkirby merged 17 commits into
puppetlabs:masterfrom
highb:2020-03-05_update_rubocop_exceptions
Mar 10, 2020
Merged

Manual Rubocop Fixes#361
mattkirby merged 17 commits into
puppetlabs:masterfrom
highb:2020-03-05_update_rubocop_exceptions

Conversation

@highb

@highb highb commented Mar 6, 2020

Copy link
Copy Markdown
Contributor

This should fix the remaining Rubocop issues that couldn't be autocorrected.

Still working on the last couple of issues, I'll remove the DNM tag when those are finished.

I assume `propSpecs` is a reference to the VMWare API so using camelCase
is intentional.
@highb highb requested a review from a team as a code owner March 6, 2020 00:31
Brandon High added 9 commits March 5, 2020 16:53
This commit fixes the various calls to `format` to consistently use the
keyword token style. Hopefully this is more understandable and explicit?
This commit:
* explicitly defines some short variable names that
generally make sense in the codebase.
* set numeric comparisons to be enforced instead of the predicate style
Prior to this commit there were a couple locations where exceptions were
saved to `_e` but weren't used in the handler except to re-raise the
exception, which simply calling the `raise` keyword will do without a
provided argument.
This commit removes the unnecessary assignment of the exception to a
variable and simply uses `raise` instead.
As far as I can tell, this variable doesn't exist before this assignment
and is immediately overwritten with the result of the method call so I
don't think it needs to be here?
This commit updates the dashboard to use `URI.parse` instead of `Kernel#open`
because open can potentially open files on the server and has other
possible security issues.

Also updated the logger to use `File.open` as it is more explicit and
doesn't have the extra potential for abuse like `Kernel#open`

https://rubocop.readthedocs.io/en/latest/cops_security/#securityopen
This commit ignores the `set_linked_clone` method, which doesn't obey
the Rubocop recommended method naming convention.
This commit updates places where previously we were casting both terms
in a float division into floats in order to ensure that float division
occurs to use the `fdiv` method, which will always do float division and
is available on both `Floats` and `Integers` because they are
both `Numeric`.
This commit fixes a bug in update_clone_target where I believe `=` was
intended, not `==` because `==` just goes to the void context here.

Thanks Rubocop Lint/Void!
@highb highb force-pushed the 2020-03-05_update_rubocop_exceptions branch from 3bdbf36 to cfc4326 Compare March 6, 2020 01:00
Brandon High added 6 commits March 5, 2020 17:10
This commit fixes this call to `dc.vmFolder.traverse` to simply pass in
the arguements instead of assigning to variables that are discarded.
Prior to this commit the codebase used the `zero?` method for comparing
to 0 on some places and not in others.
This commit makes all comparison to zero consistently use the `==`.
This commit drops the `has_` prefix from several `?` style methods
because that's against the ruby style guide and redundant for a method
ending in `?`.
This commit updates a block that was creating a shadow variable to use a
different variable.
This commit removes two duplicate return statements in both branches of
a conditional with one return statement outside the conditional blocks.
This commit switches the early `return` in `migrate_vm`'s connection
pooling block to a `break`, since `return` implies you are returning
something from a method and I don't think `migrate_vm` wants to do that.

This is a place where a partial type system like Sorbet seems useful
because then we'd have some idea about what the intended return is here.
@highb highb force-pushed the 2020-03-05_update_rubocop_exceptions branch from cfc4326 to 57d2010 Compare March 6, 2020 01:26
This commit updates Travis to require Rubocop to pass. This is possible
now due to the fixes in puppetlabs#361.
Comment thread lib/vmpooler/pool_manager.rb

@mattkirby mattkirby left a comment

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.

Looks good to me. I just have the one inline question about the change to time.

@mattkirby mattkirby merged commit 339f1db into puppetlabs:master Mar 10, 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.

2 participants