Updated Rubocop to latest version

[EtienneDepaulis]

Hello guys,

I wanted to fix a few things on your gem (we have an extended use if it as all our infrastructure is FHIR based and some of our clients are Rails apps). As we use ruby 2.5 by default, I had to update Rubocop to it's latest version. Was that the correct choice ?

I fixed a Performance/RegexpMatch offense and silenced Naming ones as you did.

Here are all the previous offenses:

With this PR, bundle exec rake test works perfectly in a 2.5.x environment.

[jawalonoski]

Thank you for the pull request. It may take us a few days to review this admittedly simple change. We need to discuss the implications of the change and assuming Ruby 2.5 as the default.

[arscan]

I generally have no problem with updating a dependency like this, as it seems like it would be a good idea to use newer versions and one would think we would have to update it eventually. But I'm confused about how Ruby 2.5 comes into play here. It appears that Rubocop has no problem running when I'm in ruby 2.5.1 on this project, using version 0.49.1 of Rubocop.

I'm not really familiar with Rubocop, but is the problem that your parent project wants to traverse this gem using he newer version of Rubocop than what we have specified, and therefore you need to update the .rubocop.yml to add the right exception? But that isn't a ruby version problem since 0.49.1 appears to work in Ruby 2.5.

@jawalonoski: Just FYI: We are good running 2.5 in general on this gem, as it is now required for crucible_smart_app for TLS testing support, and our .travis.yml is up to date with the versions we support.

[EtienneDepaulis]

@arscan I had the following issue when pulling master and setting 2.5.1 as the local ruby version:

→ bundle exec rake test
Running RuboCop...
Error: Unknown Ruby version 2.5 found in `.ruby-version`.
Known versions: 1.9, 2.0, 2.1, 2.2, 2.3, 2.4
RuboCop failed!

I checked on a few GH issues and all were suggesting to update the gem to its latest version.

Rubocop is a "development dependency" so it does not affect our apps when installing the gem :) We use it on 2.5.1 and 2.5.0 apps and older versions of rubocop without any problem.

[arscan]

Ok, that's what I was missing. Rubocop is finding a .ruby-version file in your environment that it uses to determine which version of their rule set to validate against. My environment doesn't have that file, so Rubocop defaults to the Ruby 2.1 rule set for version 0.49.1.

Thanks for pointing this out, I wasn't aware that this was a problem!

We are very lenient with the Ruby versions that we support for this gem (2.2-2.5), so we probably should be telling Rubocop to use the 2.2 style rule set, instead of the current situation where it might run against different style rules depending on the environment. To do that, we can add TargetRubyVersion: 2.2 to the .rubocop.yml, as described in the docs.

I added the .ruby-version file, which caused the error you were seeing to show up. I then updated the config file to set the style set to 2.2 (which is the oldest version we support), and it worked.

Does this seem like a reasonable alternative to your proposed solution?

[EtienneDepaulis]

@arscan seems like a perfect solution. I've updated my PR with the missing config :)
Thanks for taking the time to check this.

[arscan]

Thanks @EtienneDepaulis . I have one more thing to ask -- now that we have set the target to 2.2, I don't believe that we need to silence the Naming offense. I'd prefer to not silence that because it seems like a reasonable thing to fix at some major version update in the future, just not right away. If we silence it, it is more likely that we will forget about it, and potentially silence other errors that fall under the 'Naming' umbrella.

Could you try unsilencing that, and if it still works push that update (and I'll then merge)? Or let me know if I'm miscalculating and unsilencing it still causes the error for you.

Thanks for your help!

[EtienneDepaulis]

@arscan Here are the original errors:

The startTime and endTime are pretty obvious, the is_valid? is a breaking change so I'm not a huge fan.
The set_bearer_token and get_oauth2_metadata_from_conformance are clearly breaking changes to I silenced only the specific cop.

What do you think ?

[arscan]

Thanks for continuing to work through this with me @EtienneDepaulis -- I appreciate it.

We can't change is_valid? until a new major version, as it would be a breaking change to the API. I was hoping that by setting TargetRubyVersion: 2.2 in the .rubocop.yml, we could simply remove the lines that disabled the Naming cop, as the I thought the violations for the Naming cop came out of versions later than 2.2.

If that isn't the case, the let's just leave the Naming cop in there as it was, and I'll open an issue targeting 4.0 of this client to clean up those aspects of the API. In other words, I'm good with you reverting this last commit, and I'll merge. Make sense?

[EtienneDepaulis]

Hi @arscan, I was expecting this answer about is_valid? to be honest.
I forced removed my last commit.

Thanks for taking the time to find the best compromise :)