[Ruby] - Upgraded Rubocop gems. #342
Conversation
There was a problem hiding this comment.
Cheers! A few remarks, and I'll have to leave the actual review to @luke-hill.
| @@ -23,14 +23,14 @@ Gem::Specification.new do |s| | |||
| 'source_code_uri' => 'https://github.com/cucumber/cucumber-expressions/tree/main/ruby', | |||
| } | |||
|
|
|||
| s.add_runtime_dependency 'bigdecimal' | |||
| s.add_dependency 'bigdecimal' | |||
There was a problem hiding this comment.
@luke-hill this looks breaking. Is that correct?
There was a problem hiding this comment.
Nah this is just minor naming. Although I wasn't aware this was a new recommendation 🤷♂️
| @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/). | |||
|
|
|||
| ## [Unreleased] | |||
| ### Changed | |||
| - [Ruby] Updated rubocop and rubocop gems (RSpec/Rake/Performance), also updated project files affected by the updated rules ([#342](https://github.com/cucumber/cucumber-expressions/pull/342)) | |||
There was a problem hiding this comment.
This is not necessary. Notable in this context means "notable to the user" and "notable for the purposes of semver".
There was a problem hiding this comment.
Yeh, ideally here you'd maybe write a quick changelog that you autofixed some styles or something. As some of the changes could break, but this would go into a patch release if it was released
There was a problem hiding this comment.
Sorry, but not completely sure if I should keep the changelog entry or remove it 😅
There was a problem hiding this comment.
I think what Rien is saying is technical things about ruby don't belong in a polyglot changelog (This is a library with 6 flavours).
So maybe just say somethinig similar to what has been written prior
| @@ -23,14 +23,14 @@ Gem::Specification.new do |s| | |||
| 'source_code_uri' => 'https://github.com/cucumber/cucumber-expressions/tree/main/ruby', | |||
| } | |||
|
|
|||
| s.add_runtime_dependency 'bigdecimal' | |||
| s.add_dependency 'bigdecimal' | |||
There was a problem hiding this comment.
Nah this is just minor naming. Although I wasn't aware this was a new recommendation 🤷♂️
| @@ -33,7 +33,7 @@ def initialize(group, parameter_type) | |||
| def value(self_obj = :nil) | |||
| raise 'No self_obj' if self_obj == :nil | |||
|
|
|||
| group_values = @group ? @group.values : nil | |||
| group_values = @group&.values | |||
There was a problem hiding this comment.
This can be moved into the line below
| @@ -155,7 +155,7 @@ def ==(other) | |||
| }.to raise_error('There is already a parameter with name color') | |||
| end | |||
|
|
|||
| it 'is not detected for type' do | |||
| it 'is not detected for type', skip: 'missing expectation' do | |||
There was a problem hiding this comment.
Just flagging this for me here, this should be checked across languages. It "could" be similar issue
| @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/). | |||
|
|
|||
| ## [Unreleased] | |||
| ### Changed | |||
| - [Ruby] Updated rubocop and rubocop gems (RSpec/Rake/Performance), also updated project files affected by the updated rules ([#342](https://github.com/cucumber/cucumber-expressions/pull/342)) | |||
There was a problem hiding this comment.
Yeh, ideally here you'd maybe write a quick changelog that you autofixed some styles or something. As some of the changes could break, but this would go into a patch release if it was released
Thanks for the review! 🙌 |
🤔 What's changed?
Upgraded Rubocop and associated libraries:
rubocop-performancerubocop-rakerubocop-rspecAlso ran
rubocopwithin the relevantrubyfolder an updated files based on the new rules.🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
It does not bring any breaking change due to updating these libraries.
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.