Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MinSize option to Style/SymbolArray #4262

Merged
merged 1 commit into from
Apr 12, 2017
Merged

Add MinSize option to Style/SymbolArray #4262

merged 1 commit into from
Apr 12, 2017

Conversation

scottmatthewman
Copy link
Contributor

@scottmatthewman scottmatthewman commented Apr 11, 2017

In line with Style/WordArray, allow a MinSize setting to determine the number of items to allow per array before the cop kicks in.

The minimum size handling is extracted into a new mixin, and some other similar functionality between the two cops has been better reflected with some method renaming and reordering.


  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Used the same coding conventions as the rest of the project.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • All tests are passing.
  • The new code doesn't generate RuboCop offenses.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Updated cop documentation with rake generate_cops_documentation (required only when you've added a new cop or changed the configuration/documentation of an existing cop).

@scottmatthewman
Copy link
Contributor Author

I've only included the generate_cops_documentation for this cop in the PR – there are a few outstanding changes in master that are yet to be reflected in the docs but it'd be inappropriate to include them here.


module RuboCop
module Cop
# Common functionality for handling array count minimums
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd extend this documentation a bit, perhaps referencing the use-cases warranting the existence of the mixin. At the very least array count minimums sounds pretty vague and confusing to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point. I've expanded the documentation on the mixin to specify which configuration option it is handling, and then expanded the doc comments on both Style/SymbolArray and Style/WordArray to explain the configuration parameter.

In line with `Style/WordArray`, allow a `MinSize` setting to determine
the number of items to allow per array before the cop kicks in.

The minimum size handling is extracted into a new mixin, and some
other similar functionality between the two cops has been better
reflected with some method renaming and reordering.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants