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

Apply rubocop basic rules #3764

Merged
merged 7 commits into from
Oct 22, 2019
Merged

Apply rubocop basic rules #3764

merged 7 commits into from
Oct 22, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Oct 10, 2019

Objectives

  • Apply rules we've agreed upon to existing code
  • Have zero reports when running rubocop --config .rubocop_basic.yml --fail-level convention --display-only-fail-level-offenses

Notes

  • The file .rubocop.yml contains rules we haven't agreed upon (yet). We don't follow some of the rules defined there, and this pull request doesn't adress those rules.
  • The rule with Severity: refactor (that is, Metrics/LineLength) is not addressed in this pull request either. We might address it in the future.

@javierm javierm added the Linters Rubocop, ERB Lint, ESLint, SCSS-Lint, ... label Oct 10, 2019
@javierm javierm self-assigned this Oct 10, 2019
@javierm javierm force-pushed the sanitize_description branch from 9ae15d4 to 17ae9c9 Compare October 21, 2019 19:34
@javierm javierm changed the base branch from sanitize_description to master October 22, 2019 12:22
We've got a rubocop rule preventing us from using them, and the tests
are easier to read this way.
I was using Time.now because that's what Rails actually does, but we get
a warning by rubocop.
We were a bit inconsistent when aligning equal signs vertically.
The `find` method raises an exception if nothing is found, so there's no
need to check if it found something.
We were already using it most of the time, but not always.
Usually when we use `try` we actually mean `try!`, which is the same as
the safe navigation operator. However, there are a few cases where we
actually mean to execute a method if the object responds to that method.

In those cases using `try` would actually be OK, but in order to avoid
confusion as to whether we mean to check for `respond_to?` or we mean to
use safe navigation, I'm removing all usages of `try`.
@@ -930,7 +930,7 @@ def investments_order
scenario "Edit", :js do
daniel = create(:user, :level_two)

create(:budget_investment, heading: heading,title: "Get Schwifty", author: daniel, created_at: 1.day.ago)
create(:budget_investment, heading: heading, title: "Get Schwifty", author: daniel, created_at: 1.day.ago)

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [112/110] (https://rubystyle.guide#80-character-limits)

@javierm javierm merged commit d29d5ed into master Oct 22, 2019
@javierm javierm deleted the rubocop_basic branch October 22, 2019 16:49
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linters Rubocop, ERB Lint, ESLint, SCSS-Lint, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants