Update rubocop-govuk to 4.0.0.pre.1 #1034
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is to test the pre-release out of 4.0.0. Further details are in: alphagov/rubocop-govuk#129
rubocop -A
These can't be fixed without causing a backwards compatibility issue. I've chose to disable these rules with inline comments as we want this cop to apply for new code. It is likely that the performance platform aspect of this code will be removed soon (the platform is due for retirement) which will just leave the one offence - if I remember I'll see if it can be removed on the next major release.
This was used to shorten a class definition, I decided that this class name wasn't too long so the simplest solution was to repeat it. Alternative approaches seemed less idiomatic (for example defining a variable like `cache_control_klass`).
We can't fix this violation and maintain the ability to do `require "gds-api-adapters"` so I've updated this message to reflect that.
This updates the required ruby version of this gem to be 2.6 and sets the .ruby-version to match. This has been done to reflect that this gem is only tested with 2.6 and 2.7 [1]. To avoid complications in development and linting the same minor Ruby version has been used for both, hence the regression from 2.7 to 2.6 in the .ruby-version file. [1]: https://github.com/alphagov/govuk-jenkinslib/blob/d5deab4438d803954ef13f94a2cfa528290932d2/vars/govuk.groovy#L617
rubocop -A
This is a breaking change however from my brief research (searching my machine) it doesn't seem that this is argument is used outside of this utility. Therefore, this seems to be quite a benign breaking change.
This seems to be a widely spread convention we have that no-one seems to know why. Removing it makes life a little simpler.
leenagupte
reviewed
Feb 26, 2021
| @@ -7,7 +7,7 @@ module TestHelpers | |||
| module ContentStore | |||
| include ContentItemHelpers | |||
|
|
|||
| def content_store_endpoint(draft = false) | |||
| def content_store_endpoint(draft = false) # rubocop:disable Style/OptionalBooleanParameter | |||
There was a problem hiding this comment.
So the linter wants this changed to def content_store_endpoint(draft: false)?
There was a problem hiding this comment.
Yup that's correct. On the basis that method calls like content_store_endpoint(true) don't really mean anything but content_store_endpoint(draft: true) does.
I've fixed this as a breaking change in 4a7954c (though I don't think it's a breaking change that'll affect any apps as I can't find any apps - outside of this one - that use)
| it "takes no args and initializes with an empty set of values" do | ||
| cache_control = CacheControl.new | ||
| cache_control = GdsApi::Response::CacheControl.new |
There was a problem hiding this comment.
Does described_class only work with RSpec?
There was a problem hiding this comment.
Yes, sadly so: minitest/minitest#526
| @@ -395,7 +395,7 @@ def test_client_can_set_custom_headers_on_gets | |||
| ) | |||
|
|
|||
| assert_requested(:get, %r{/some.json}) do |request| | |||
| headers_with_uppercase_names = Hash[request.headers.collect { |key, value| [key.upcase, value] }] | |||
| headers_with_uppercase_names = request.headers.transform_keys(&:upcase) | |||
There was a problem hiding this comment.
Well this is much nicer!
leenagupte
approved these changes
Mar 1, 2021
|
Thanks @leenagupte 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trello: https://trello.com/c/pqXbKZiC/215-test-rubocop-govuk-400pre1-prior-to-400-release
This bumps to the 4.0.0 pre release of rubocop-govuk which is part of trialling the new rubocop-govuk release. The gem pin to 4.0.0.pre.1 is intended to be removed once the final release of rubocop-govuk 4.0.0 is out.
Applying this revealed we were supporting a rather old Ruby version (2.4) so I've bumped our support to 2.6 and marked this as a breaking change.