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

Update rubocop and re-lint files #473

Merged
merged 2 commits into from
Apr 24, 2019
Merged

Update rubocop and re-lint files #473

merged 2 commits into from
Apr 24, 2019

Conversation

KnVerey
Copy link
Contributor

@KnVerey KnVerey commented Apr 24, 2019

What are you trying to accomplish with this PR?
Update our rubocop compliance. I noticed that it was correcting untouched code when I ran it on my last PR.

How is this accomplished?
gem install rubocop, delete the styleguide cache, rubocop -a and then fix what couldn't be auto-corrected.

What could go wrong?
Committing unintentional changes, because I actually did this on my other branch and pulled it over from there.

@Shopify/cloudx

@@ -53,7 +53,7 @@ def build(namespace:, context:, definition:, logger:, statsd_tags:, crd: nil)

def class_for_kind(kind)
if KubernetesDeploy.const_defined?(kind)
KubernetesDeploy.const_get(kind) # rubocop:disable Sorbet/ConstantsFromStrings
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rubocop locally complains that disabling this cop has no effect, but then Policial fails without it. I added a general exclusion to the main rubocop config instead. We're not using Sorbet.

Copy link
Contributor

@dturn dturn left a comment

Choose a reason for hiding this comment

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

1 question, since its all auto-generated fixes I'm not reviewing style

@@ -399,7 +399,7 @@ def dummy_events(start_time)
count: 3,
last_seen: start_time + 3.seconds,
reason: "FailedSync",
message: <<~STRING
message: <<~STRING,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really where the , goes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep--it's like when you chain a method on a heredoc, this is where it goes (<<~STRING.strip for example). I agree this comma looks stupid though. 😞

@KnVerey KnVerey merged commit 606eee1 into master Apr 24, 2019
@KnVerey KnVerey temporarily deployed to rubygems April 29, 2019 17:38 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.

2 participants