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

[Fix #3631] Add cop to check for spaces in '-> (' format lambdas #3656

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@swcraig
Contributor

swcraig commented Oct 20, 2016

I've created the cop with two configuration settings, require_space and require_no_space.
The default is require_no_space.


Before submitting the PR make sure the following are checked:

  • 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).

astorije added a commit to astorije/graphql-ruby that referenced this pull request Oct 21, 2016

@Drenmi

This could be personal opinion, but I would normally call it a "lambda literal." This could maybe help clear up some of the naming and error messages. 😀

Also, I know that we extensively used the convention naming arguments to private methods just node (and I have done that a lot as well), but I really think we should start being a bit more descriptive than that when a particular node is expected. WDYT? 😅

Show outdated Hide outdated CHANGELOG.md Outdated
Show outdated Hide outdated lib/rubocop/cop/style/space_after_lambda_arrow.rb Outdated
@mikegee

This comment has been minimized.

Show comment
Hide comment
@mikegee

mikegee Oct 21, 2016

Contributor

I agree with "lambda literal." -> is special syntax that Ruby interprets like a call to lambda, just like quotes and brackets are special syntax for string, array, and hash literals that Ruby interprets as calls to String.new, etc.

Contributor

mikegee commented Oct 21, 2016

I agree with "lambda literal." -> is special syntax that Ruby interprets like a call to lambda, just like quotes and brackets are special syntax for string, array, and hash literals that Ruby interprets as calls to String.new, etc.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 24, 2016

Collaborator

I merged this manually.

A small note - not sure whether this cop should warn for multiple spaces between -> and (. Some people might be using them for alignment, but it seems like a bad idea to me.

Collaborator

bbatsov commented Oct 24, 2016

I merged this manually.

A small note - not sure whether this cop should warn for multiple spaces between -> and (. Some people might be using them for alignment, but it seems like a bad idea to me.

@bbatsov bbatsov closed this Oct 24, 2016

@swcraig swcraig deleted the swcraig:lambda-arrow-cop branch Oct 24, 2016

@swcraig

This comment has been minimized.

Show comment
Hide comment
@swcraig

swcraig Oct 24, 2016

Contributor

@bbatsov Thanks.
I am still relatively new to this workflow. In what cases would you manually merge a PR? Is it just when you would like to test the changes locally on your machine? Or are there other cases?

Contributor

swcraig commented Oct 24, 2016

@bbatsov Thanks.
I am still relatively new to this workflow. In what cases would you manually merge a PR? Is it just when you would like to test the changes locally on your machine? Or are there other cases?

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 24, 2016

Collaborator

When there are conflicts in the changelog and I don't want to bother the authors to rebase.

Collaborator

bbatsov commented Oct 24, 2016

When there are conflicts in the changelog and I don't want to bother the authors to rebase.

@astorije

This comment has been minimized.

Show comment
Hide comment
@astorije

astorije Oct 25, 2016

Contributor

Thanks for this! Any plans for a release yet? :)

I don't want to bother the authors to rebase.

Btw, you can now edit the PR author branch (add commits, rebase existing commits) assuming the PR is set with edit rights to maintainers (it is by default).
It is roughly the same amount of steps, but with the appreciable perk that you don't have to close the PR.

Contributor

astorije commented Oct 25, 2016

Thanks for this! Any plans for a release yet? :)

I don't want to bother the authors to rebase.

Btw, you can now edit the PR author branch (add commits, rebase existing commits) assuming the PR is set with edit rights to maintainers (it is by default).
It is roughly the same amount of steps, but with the appreciable perk that you don't have to close the PR.

astorije added a commit to astorije/graphql-ruby that referenced this pull request Oct 25, 2016

astorije added a commit to astorije/graphql-ruby that referenced this pull request Oct 25, 2016

astorije added a commit to astorije/graphql-ruby that referenced this pull request Oct 25, 2016

rmosolgo added a commit to astorije/graphql-ruby that referenced this pull request Mar 3, 2017

riffraff added a commit to riffraff/graphql-ruby that referenced this pull request Jun 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment