Fix Style/BlockDelimiters #1224

mxygem · 2017-10-28T02:53:05Z

Details

Rubocop prefers the use of {...} for single line blocks and do..end for multi-line blocks.

Motivation and Context

Working to help solve issue 1021!

How Has This Been Tested?

bundle exec rake 👍

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Rubocop style fixes

luke-hill

Couple of syntatical observations

luke-hill · 2017-11-24T08:55:19Z

lib/cucumber/formatter/junit.rb

@@ -25,15 +25,16 @@ def initialize(config)
        config.on_event :test_run_finished, &method(:on_test_run_finished)
        @reportdir = ensure_dir(config.out_stream, 'junit')
        @config = config
-        @features_data = Hash.new { |h,k| h[k] = {
+        @features_data = Hash.new do |h,k| h[k] = {


whilst not in this cop, probably worth doing |h, k| just whilst you're here. Also line 35 can go to new syntax if you're inclined

All set on this. :)

luke-hill · 2017-11-24T08:56:11Z

spec/cucumber/filters/tag_limits/verifier_spec.rb

@@ -49,9 +49,9 @@
      end

      it 'does not raise an error' do
-        expect {
+        expect do


possible 1 liner?

luke-hill · 2017-11-24T08:57:05Z

spec/cucumber/formatter/json_spec.rb

@@ -550,8 +550,8 @@ module Formatter
            FEATURE

          define_steps do
-            Given(/^there are bananas$/) { data = 'YWJj'
-              embed data, 'mime-type;base64' }
+            Given(/^there are bananas$/) do data = 'YWJj'


Like below, Given end typically are your wrappers, so I'd have them distinct. Move end:554 to 555?

That's a weird one- don't know how that happened! Nice catch!

For some weird reason the json_spec file seems very fragile and without that extra line under my change, some of the tests following that one fail.

luke-hill · 2017-11-24T08:57:59Z

spec/cucumber/rake/forked_spec.rb

@@ -36,8 +36,8 @@ module Rake
      context 'when running without bundler' do
        let(:bundler) { false }

-        subject { Task::ForkedCucumberRunner.new(
-            libs, binary, cucumber_opts, bundler, feature_files) }
+        subject do Task::ForkedCucumberRunner.new(


do and end are the starting arguments and distinct like above. I'll leave the rest out.

All set on this one

mxygem · 2018-01-07T18:47:23Z

Will work to fix conflicts today if I can! Also, @luke-hill Thanks for the feedback, I'll also work on your changes when I get to this PR. :)

luke-hill · 2018-01-09T11:12:14Z

spec/cucumber/formatter/json_spec.rb

-            Given(/^there are bananas$/) { data = 'YWJj'
-              embed data, 'mime-type;base64' }
+            Given(/^there are bananas$/) { embed('YWJj', 'mime-type;base64') }
+


@luke-hill This line has to be here otherwise random tests after this one fail. Seems to be a really fragile file. :( We can work on fixing that in another issue.

luke-hill · 2018-01-09T11:12:42Z

spec/cucumber/rake/forked_spec.rb

-        subject { Task::ForkedCucumberRunner.new(
-            libs, binary, cucumber_opts, bundler, feature_files) }
+        subject do Task::ForkedCucumberRunner.new(
+            libs, binary, cucumber_opts, bundler, feature_files) end


think the end here should be newlined?

I can try new lining it, but given my comment on json_spec, it may break things. Technically, it should just be a one-line request, but I think it's on two due to line-length. :grumble:

Nevermind, things are fine after changing.

olleolleolle · 2018-01-10T08:06:43Z

spec/cucumber/formatter/html_spec.rb

          @formatter.feature_name('Feature', '')
-        }).not_to raise_error
+        end).not_to raise_error


One way to get this to read a little easier is to try keeping the "stabby lambda" and having it on 1 line:

expect(->{ @formatter.feature_name('Feature', '') }).not_to raise_error

All set here!

olleolleolle · 2018-01-10T08:07:31Z

spec/cucumber/formatter/interceptor_spec.rb

          Interceptor::Pipe.wrap(:nonsense)
-        }.to raise_error(ArgumentError)
+        end.to raise_error(ArgumentError)


Perhaps this also reads easy on 1 line?

expect { Interceptor::Pipe.wrap(:nonsense) }.to raise_error(ArgumentError)

All set here!

olleolleolle · 2018-01-10T08:07:53Z

spec/cucumber/formatter/interceptor_spec.rb

          Interceptor::Pipe.unwrap!(:nonsense)
-        }.to raise_error(ArgumentError)
+        end.to raise_error(ArgumentError)


And, this too. You see what I'm trying to achieve, I hope.

Yeah, I totally do. The project looks like it may need a larger rspec update to bring it up to speed stylistically. Thanks!

All set here!

olleolleolle · 2018-01-10T08:09:43Z

spec/cucumber/formatter/junit_spec.rb

              run_defined_feature
-            }).to raise_error(Junit::UnNamedFeatureError)
+            end).to raise_error(Junit::UnNamedFeatureError)


Oh, this would also read easier to me as a single-line

expect(->{ run_defined_feature }).to raise_error(Junit::UnNamedFeatureError)

olleolleolle · 2018-01-10T08:12:55Z

Oh, blast. So many instances of the same thing.

Where I'm trying to go is: The two forms of anonymous methods in Ruby can stylistically be used in two distinct forms: as inline, or as multiline block.

stabby lambda: use it inline ->(arg){}
regular lambda: use it with a do block: lambda do |arg| multi-line content here... end

luke-hill · 2018-01-10T09:22:12Z

Yeh thats a good point ☝️ Wasn't sure how you've been tackling these whether manually or through some rubocop -a calls. But I think the above is invoked through -a

mxygem · 2018-01-16T18:49:21Z

Thanks, you two! I'll get these handled. :)

mxygem · 2018-01-17T06:11:13Z

spec/cucumber/multiline_argument/data_table_spec.rb

@@ -88,17 +88,17 @@ module MultilineArgument
        end

        it 'should pass silently if a mapped column does not exist in non-strict mode' do
-          expect {
+          expect do


Leaving these as they're multi-line

mxygem · 2018-01-17T17:05:00Z

@olleolleolle Are we good to go here? :D

olleolleolle · 2018-01-17T17:07:55Z

I'm going to merge this, now! 👍

mxygem · 2018-01-17T17:11:13Z

Woot! Thanks!

lock · 2019-01-17T20:29:24Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Fix Style/BlockDelimiters

2ccb87a

ghost assigned mxygem Oct 28, 2017

ghost added the status: work in progress (WIP) label Oct 28, 2017

mxygem added status: ready and removed status: work in progress (WIP) labels Oct 28, 2017

Changelog update

729a1a5

ghost added status: work in progress (WIP) and removed status: ready labels Oct 28, 2017

mxygem added status: ready and removed status: work in progress (WIP) labels Oct 28, 2017

Merge branch 'master' into 1021-BlockDelimiters

63969ad

ghost added status: work in progress (WIP) and removed status: ready labels Nov 24, 2017

luke-hill reviewed Nov 24, 2017

View reviewed changes

aslakhellesoy removed the status: work in progress (WIP) label Nov 28, 2017

Minor changes to bring things inline with PR comments

232bfc3

ghost added the status: work in progress (WIP) label Jan 8, 2018

Merge master and fix conflict

03f2550

luke-hill reviewed Jan 9, 2018

View reviewed changes

Fix line arrangement

4aab777

olleolleolle reviewed Jan 10, 2018

View reviewed changes

mxygem commented Jan 17, 2018

View reviewed changes

Updating syntax of functions and one-lining where I can

bfbba34

mxygem removed the status: work in progress (WIP) label Jan 17, 2018

olleolleolle merged commit 8a109fc into master Jan 17, 2018

olleolleolle deleted the 1021-BlockDelimiters branch January 17, 2018 17:08

dependabot-preview bot mentioned this pull request Mar 20, 2018

Bump cucumber from 2.4.0 to 3.1.0 alphagov/whitehall#3888

Closed

lock bot locked as resolved and limited conversation to collaborators Jan 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Style/BlockDelimiters #1224

Fix Style/BlockDelimiters #1224

mxygem commented Oct 28, 2017

luke-hill left a comment

luke-hill Nov 24, 2017

mxygem Jan 8, 2018 •

edited

Loading

luke-hill Nov 24, 2017

mxygem Jan 8, 2018

luke-hill Nov 24, 2017

mxygem Jan 7, 2018

mxygem Jan 8, 2018

luke-hill Nov 24, 2017

mxygem Jan 8, 2018

mxygem commented Jan 7, 2018

luke-hill Jan 9, 2018

mxygem Jan 9, 2018

luke-hill Jan 9, 2018

mxygem Jan 9, 2018

mxygem Jan 9, 2018

olleolleolle Jan 10, 2018

mxygem Jan 17, 2018

olleolleolle Jan 10, 2018

mxygem Jan 17, 2018

olleolleolle Jan 10, 2018

mxygem Jan 16, 2018

mxygem Jan 17, 2018

olleolleolle Jan 10, 2018

olleolleolle commented Jan 10, 2018

luke-hill commented Jan 10, 2018

mxygem commented Jan 16, 2018

mxygem Jan 17, 2018

mxygem commented Jan 17, 2018

olleolleolle commented Jan 17, 2018

mxygem commented Jan 17, 2018

lock bot commented Jan 17, 2019

Fix Style/BlockDelimiters #1224

Fix Style/BlockDelimiters #1224

Conversation

mxygem commented Oct 28, 2017

Details

Motivation and Context

How Has This Been Tested?

Types of changes

luke-hill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxygem Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxygem commented Jan 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olleolleolle commented Jan 10, 2018

luke-hill commented Jan 10, 2018

mxygem commented Jan 16, 2018

Choose a reason for hiding this comment

mxygem commented Jan 17, 2018

olleolleolle commented Jan 17, 2018

mxygem commented Jan 17, 2018

lock bot commented Jan 17, 2019

mxygem Jan 8, 2018 •

edited

Loading