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

Allow to as method parameter in default config #5626

Merged

Conversation

unused
Copy link
Contributor

@unused unused commented Mar 5, 2018

Add to to AllowedNames in default configuration for cop
Naming/UncommunicativeMethodParamName in order to allow common
methods handling date ranges with parameter set_date_range(from, to)

CHANGELOG.md Outdated
@@ -2,6 +2,9 @@

## master (unreleased)

### Changes
* Change `Naming/UncommunicativeMethodParamName` add `to` to allowed names in default config. ([@unused][])
Copy link
Member

Choose a reason for hiding this comment

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

Please write the link to this PR at the beginning 😃

- * Change `Naming/UncommunicativeMethodParamName` add `to` to allowed names in default config. ([@unused][])
+ * [#5626](https://github.com/bbatsov/rubocop/pull/5626): Change `Naming/UncommunicativeMethodParamName` add `to` to allowed names in default config. ([@unused][])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry and thx 😄

@Drenmi
Copy link
Collaborator

Drenmi commented Mar 5, 2018

Would it be meaningful to add some common preposition, such as "in", "on", "at", etc.?

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 6, 2018

Perhaps, although I've rarely seen those as method names.

@pocke
Copy link
Collaborator

pocke commented Mar 6, 2018

Would it be meaningful to add some common preposition, such as "in", "on", "at", etc.?

Yeah, but in is keyword of Ruby (for var in collection syntax) so we cannot use in as method parameter name.

@pocke
Copy link
Collaborator

pocke commented Mar 6, 2018

Can you check the failing build on Travis CI?
https://travis-ci.org/bbatsov/rubocop/jobs/349272326#L1387-L1388
Probably we should fix the document generator task. Probably it seems confusing with an empty value.

@unused
Copy link
Contributor Author

unused commented Mar 6, 2018

I cannot think of an example using "in", "on", "at" except using some as pre-/suffixes. Happy to add if someone provides a legit code example :)

@unused
Copy link
Contributor Author

unused commented Mar 6, 2018

I manually adapted the documentation, did not know about the generation task - did I miss it in the contribution guide? If it is missing, it might be a good hint to have.

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 8, 2018

It's in the PR template, but probably you didn't notice it there.

Can you rebase on top of master?

unused added 3 commits March 8, 2018 13:14
Add `to` to AllowedNames in default configuration for cop
Naming/UncommunicativeMethodParamName in order to allow common
methods handling date ranges with parameter `set_date_range(from, to)`
Fixes issue with whitespace in docs, see rubocop#5626.
@unused unused force-pushed the allow-short-method-param-to-in-default-config branch from f36647f to 0228bb5 Compare March 8, 2018 12:19
@unused
Copy link
Contributor Author

unused commented Mar 8, 2018

Thx, I'll double check next time.

Rebased.

@deivid-rodriguez
Copy link
Contributor

What about ip? Can we get that added too?

@bbatsov bbatsov merged commit a43d904 into rubocop:master Mar 8, 2018
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 8, 2018

Sure.

This was referenced Mar 21, 2018
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.

6 participants