Fixes rubocop violation Style/IndentArray #1045

Open
wants to merge 2 commits into
from

Projects

None yet

3 participants

@pmatsinopoulos

Summary

Styling changes that have to do with array indentation.

Details

Rubocop complaints if indentation of items of arrays are not as it expects them to be based on Style/IndentArray cop. So, I have fixed the indentation to be compatible with rubocop.

Motivation and Context

This change is required if we want to comply with rubocop rules.

Refs #1021

How Has This Been Tested?

I have run all the tests with bundle exec rake and everything was green.

Screenshots (if appropriate):

N/A

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)
  • Style Changes

Checklist:

  • I've added tests for my code
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have only changes the code style to be compatible with Style/IndentArray cop.
@nodo

Please, can you double check the indentation in the tests? The rest looks good to me, great stuff!

+ :scenario_name,
+ :after_feature_element,
+ :after_feature,
+ :after_features,
]
@nodo
nodo Oct 27, 2016 Contributor

Just wondering, is this indentation correct?

@pmatsinopoulos
pmatsinopoulos Oct 28, 2016

@nodo, the error that this PR fixes is related to this violation message:

spec/cucumber/formatter/legacy_api/adapter_spec.rb:162:15: C: Use 2 spaces for indentation in an array, relative to the start of the line where the left square bracket is.
              :before_features,
              ^^^^^^^^^^^^^^^^

So, I avoided doing any more changes other than the ones minimum needed to get rid of this violation 🙂
Moving the indentation 2 spaces to the left (for the whole block) fixed that. Do you think that I should work on this with further enhancements? If yes, what enhancements would you suggest?

@nodo
nodo Nov 8, 2016 Contributor

Hi @pmatsinopoulos, sorry for the late reply. Makes sense! It would be great to fix the indentation as well. I that all the elements of the array should be aligned with an indentation of 2 spaces. What do you think?

@pmatsinopoulos
pmatsinopoulos Nov 9, 2016

Hi @nodo . I do not have problem to ident the way you are suggesting. However, I think that the person who has picked up such indentation did that to make the reading easier. It seems that the chosen nesting has some kind of logic, representative of the semantics of the symbols in the context they are used. I am not sure what logic is that, but it looks to me that something is going on there that makes the reading easier. Do you want to check with @mattwynne (I see the latest commits are his) before we actually go ahead and flat the indentation on those long lists of elements?

@nodo
nodo Nov 9, 2016 Contributor

Absolutely :) thanks @pmatsinopoulos !

@mattwynne
mattwynne Feb 15, 2017 edited Member

Yes, this indentation is there to make the tests easier to read / write, since you need matching pairs of those messages. I'd be happy to see it disappear though if it's a pain - I hope we'll be able to remove this entire file soon.

@mattwynne
mattwynne Feb 15, 2017 Member

Certainly, moving the indentation two spaces to the left would be fine!

@phoebeclarke phoebeclarke referenced this pull request Oct 28, 2016
Open

Fixes for rubocop violations #1048

1 of 7 tasks complete
Panayotis Ma... added some commits Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment