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

Enable remaining disabled Rubocop rules and fix violations #2549

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
inherit_from:
- .rubocop_todo.yml
AllCops:
Exclude:
- 'tmp/**/*'
120 changes: 0 additions & 120 deletions .rubocop_todo.yml

This file was deleted.

5 changes: 3 additions & 2 deletions app/models/world_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ def self.reset_cache

def self.all
cache_fetch("all") do
Services.worldwide_api.world_locations.with_subsequent_pages.map do |l|
world_locations = Services.worldwide_api.world_locations.with_subsequent_pages.map do |l|
new(l) if l.format == "World location" && l.details && l.details.slug.present?
end.compact
end
world_locations.compact
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/presenters/flow_registration_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def description

module MethodMissingHelper
def method_missing(method, *_args, &_block)
MethodMissingObject.new(method, parent_method = nil, blank_to_s = true)
MethodMissingObject.new(method, nil, true)
end
end

Expand Down
12 changes: 1 addition & 11 deletions doc/rubocop.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,7 @@ We're using the `govuk-lint` Gem to include Rubocop and the GOV.UK styleguide ru

### Jenkins

We run `govuk-lint-ruby` as part of the test suite executed on Jenkins (see jenkins.sh). Behind the scenes this runs Rubocop with a set of cops defined in the `govuk-lint` gem. However, because `govuk-lint` was retro-fitted to the application relatively recently, there were a large number of cop violations in the codebase.

We've dealt with this by generating a project-specific `.rubocop_todo.yml` file using `govuk-lint-ruby --auto-gen-config --exclude-limit 99999` which [excludes existing violations on a per-cop basis][1]. The limit was set to `99999` to minimise the number of cops that are disabled entirely - most are only disabled for a specific set of files.

The generated `.rubocop_todo.yml` file is included into the configuration via a local `.rubocop.yml` file using an `inherit_from` key. This means that the exclusions in `.rubocop_todo.yml` are used for all `govuk-lint-ruby` commands by default. Note that the `.rubocop.yml` file does not override the cop definitions in the `govuk-lint` gem - it is merely a way to add extra configuration.

Using this approach should prevent us from introducing new violations into the project without needing to fix all the existing violations first. However, in the long run, the idea is for us to gradually remove these exclusions to bring the project fully in line with the default `govuk-lint-ruby` configuration. Ultimately we will be able to remove the `.rubocop_todo.yml` & `.rubocop.yml` files and just rely on the default `govuk-lint` configuration.

Note that as per the header comment in `.rubocop_todo.yml`, changes in the inspected code, or installation of new versions of RuboCop, may require `.rubocop_todo.yml` to be generated again.
We run `govuk-lint-ruby` as part of the test suite executed on Jenkins (see jenkins.sh). Behind the scenes this runs Rubocop with a set of cops defined in the `govuk-lint` gem.

### Running locally

Expand All @@ -33,5 +25,3 @@ Testing for violations in code staged and committed locally that's not present i
```bash
$ govuk-lint-ruby --diff --cached
```

[1]: https://github.com/bbatsov/rubocop/blob/master/README.md#automatically-generated-configuration
31 changes: 0 additions & 31 deletions lib/data/finance_weighting.rb

This file was deleted.

171 changes: 0 additions & 171 deletions lib/data/which_finance_data.yml

This file was deleted.

5 changes: 3 additions & 2 deletions lib/friendly_time_diff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ def whole_months_away

def month_difference
m = 0
begin
loop do
m += 1
end while (from_date >> m) <= to_date
break unless (from_date >> m) <= to_date
end
m - 1
end

Expand Down
Loading