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

Change basic rubocop rules #3636

Merged
merged 13 commits into from
Sep 10, 2019
Merged

Change basic rubocop rules #3636

merged 13 commits into from
Sep 10, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Jun 24, 2019

References

Objectives

  • Move some of our "advanced" rubocop rules to the "basic" rules file, so they're detected by Hound
  • Apply these rules to existing code

@@ -56,7 +56,7 @@ def census_api_response
def fill_stats_fields
if in_census?
self.gender = census_api_response.gender
self.geozone_id = Geozone.select(:id).where(census_code: census_api_response.district_code).first.try(:id)
self.geozone_id = Geozone.select(:id).where(census_code: census_api_response.district_code).first&.id

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. [111/110] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

@@ -25,7 +25,7 @@ def show
.where.not(description: "").order(:given_order)

@answers_by_question_id = {}
poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user.try(:id))
poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user&.id)

Choose a reason for hiding this comment

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

Lint/UselessAssignment: Useless assignment to variable - poll_answers. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars)

@@ -18,7 +18,7 @@
scenario "The username links to their public profile" do
click_link @user.name

expect(current_path).to eq(user_path(@user))
expect(page).to have_current_path(user_path(@user))

Choose a reason for hiding this comment

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

RSpec/InstanceVariable: Use let instead of an instance variable. (http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/InstanceVariable)

@javierm javierm force-pushed the change_basic_rubocop_rules branch 2 times, most recently from be81033 to 60949bb Compare June 24, 2019 13:39
@javierm javierm self-assigned this Jun 24, 2019
@javierm javierm force-pushed the change_basic_rubocop_rules branch 2 times, most recently from 4a68574 to 654b974 Compare July 5, 2019 00:30
@javierm javierm force-pushed the add_spacing_rules branch 2 times, most recently from 0a1e7ef to 577d85d Compare July 5, 2019 01:26
@javierm javierm force-pushed the change_basic_rubocop_rules branch 2 times, most recently from d519471 to b6fd326 Compare July 5, 2019 01:30
@javierm javierm force-pushed the change_basic_rubocop_rules branch from b6fd326 to b11c5aa Compare July 5, 2019 01:38
@javierm javierm force-pushed the change_basic_rubocop_rules branch from b11c5aa to f72301a Compare July 5, 2019 03:25
@javierm javierm force-pushed the change_basic_rubocop_rules branch from f72301a to 2406de8 Compare July 5, 2019 03:44
@javierm javierm force-pushed the change_basic_rubocop_rules branch from 2406de8 to 39ba010 Compare July 5, 2019 12:57
@javierm javierm force-pushed the add_spacing_rules branch 2 times, most recently from 987099c to d0bd2b8 Compare August 8, 2019 06:40
@javierm javierm added this to Testing in Roadmap Sep 10, 2019
@javierm javierm force-pushed the add_spacing_rules branch 2 times, most recently from c13e709 to f9ed186 Compare September 10, 2019 19:05
@javierm javierm changed the base branch from add_spacing_rules to master September 10, 2019 19:42
We use staging and preproduction environments, which are not valid by
default.

This rule is useful because misspelling the name of an environment might
otherwise go unnoticed.
We're not using the InsecureProtocolSource rule because I don't feel
it's necessary.
These are rules we were already applying.

We've excluded the `factories` folder for some rules because there's a
factory defining a `context` attribute, which rubocop thought was the
`context` RSpec keyword.
@javierm
Copy link
Member Author

javierm commented Sep 10, 2019

Travis failure is not related to this pull request.

@javierm javierm merged commit 274fd1a into master Sep 10, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Sep 10, 2019
@javierm javierm deleted the change_basic_rubocop_rules branch September 10, 2019 20:27
@javierm javierm added Refactoring Linters Rubocop, ERB Lint, ESLint, SCSS-Lint, ... and removed Refactoring labels Sep 11, 2019
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
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

None yet

3 participants