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

[Fix #7064] Multiline when then cop #7114

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@okuramasafumi
Copy link
Contributor

commented Jun 6, 2019

This cop check the use of the then keyword in multi-line when
statements.
Its name comes from Style/MultilineIfThen cop.
This cop is enabled by default because Style/MultilineIfThen cop is
enabled by default.
Related issue is #7064


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).
  • 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.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch from 57c53a4 to ede08cf Jun 6, 2019

Show resolved Hide resolved CHANGELOG.md Outdated
@koic

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Style/MultilineIfThen cop is based on the Ruby Style Guide.
https://github.com/rubocop-hq/ruby-style-guide#no-then

Only if / unless is mentioned above. when should be added to it. I think this change is appropriate if accepted there.

Show resolved Hide resolved config/default.yml Outdated

okuramasafumi added a commit to okuramasafumi/ruby-style-guide that referenced this pull request Jun 11, 2019

Add `when` description/example to `no-then` rule
There's a PR to add a cop named `MultilineWhenThen`,
which detects `then` in multiline `when`.
rubocop-hq/rubocop#7114
It refers to `MultilineIfThen` cop, which is based on `no-then` rule here.
So this commit adds `when` description and example to `no-then` rule,
making newly added `MultilineWhenThen` cop based on the style guide.
FYI, the discussion about this feature is here: rubocop-hq/rubocop#7064

okuramasafumi added a commit to okuramasafumi/ruby-style-guide that referenced this pull request Jun 11, 2019

Add `when` description/example to `no-then` rule
There's a PR to add a cop named `MultilineWhenThen`,
which detects `then` in multiline `when`.
rubocop-hq/rubocop#7114
It refers to `MultilineIfThen` cop, which is based on `no-then` rule here.
So this commit adds `when` description and example to `no-then` rule,
making newly added `MultilineWhenThen` cop based on the style guide.
FYI, the discussion about this feature is here: rubocop-hq/rubocop#7064

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch 2 times, most recently from e04bb76 to 9aaaddd Jun 11, 2019

okuramasafumi added a commit to okuramasafumi/ruby-style-guide that referenced this pull request Jun 12, 2019

Add `when` description/example to `no-then` rule
There's a PR to add a cop named `MultilineWhenThen`,
which detects `then` in multiline `when`.
rubocop-hq/rubocop#7114
It refers to `MultilineIfThen` cop, which is based on `no-then` rule here.
So this commit adds `when` description and example to `no-then` rule,
making newly added `MultilineWhenThen` cop based on the style guide.
FYI, the discussion about this feature is here: rubocop-hq/rubocop#7064

okuramasafumi added a commit to okuramasafumi/ruby-style-guide that referenced this pull request Jun 12, 2019

Add `when` description/example to `no-then` rule
There's a PR to add a cop named `MultilineWhenThen`,
which detects `then` in multiline `when`.
rubocop-hq/rubocop#7114
It refers to `MultilineIfThen` cop, which is based on `no-then` rule here.
So this commit adds `when` description and example to `no-then` rule,
making newly added `MultilineWhenThen` cop based on the style guide.
FYI, the discussion about this feature is here: rubocop-hq/rubocop#7064

bbatsov added a commit to rubocop-hq/ruby-style-guide that referenced this pull request Jun 12, 2019

Add `when` description/example to `no-then` rule
There's a PR to add a cop named `MultilineWhenThen`,
which detects `then` in multiline `when`.
rubocop-hq/rubocop#7114
It refers to `MultilineIfThen` cop, which is based on `no-then` rule here.
So this commit adds `when` description and example to `no-then` rule,
making newly added `MultilineWhenThen` cop based on the style guide.
FYI, the discussion about this feature is here: rubocop-hq/rubocop#7064

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch 2 times, most recently from be6fae2 to 6734aca Jun 16, 2019

Show resolved Hide resolved config/default.yml Outdated

koic added a commit to koic/rubocop that referenced this pull request Jun 17, 2019

Add a period at EOL of description
This PR detects problems that have been reviewed several times as follows.

- rubocop-hq#7114 (comment)
- rubocop-hq#7140 (comment)

And this PR fixes existing descriptions found using this added spec.

The following is an example of detection.

```console
% bundle exec rspec spec/project_spec.rb:30
Run options: include {:focus=>true, :locations=>{"./spec/project_spec.rb"=>[30]}}

Randomized with seed 57903
F

Failures:

  1) RuboCop Project default configuration file have a period at EOL of description
    Failure/Error: expect(description).to match(/\.\z/)

     expected "Checks for flip-flops" to match /\.\z/
     Diff:
     @@ -1,2 +1,2 @@
     -/\.\z/
     +"Checks for flip-flops"

     # ./spec/project_spec.rb:34:in `block (4 levels) in <top (required)>'
     # ./spec/project_spec.rb:31:in `each'
     # ./spec/project_spec.rb:31:in `block (3 levels) in <top (required)>'

Finished in 0.07499 seconds (files took 1.61 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/project_spec.rb:30 # RuboCop Project default configuration
file have a period at EOL of description

Randomized with seed 57903
```

@koic koic referenced this pull request Jun 17, 2019

Merged

Add a period at EOL of description #7148

6 of 8 tasks complete

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch from 6734aca to afa3a6c Jun 17, 2019

bbatsov added a commit that referenced this pull request Jun 17, 2019

Add a period at EOL of description
This PR detects problems that have been reviewed several times as follows.

- #7114 (comment)
- #7140 (comment)

And this PR fixes existing descriptions found using this added spec.

The following is an example of detection.

```console
% bundle exec rspec spec/project_spec.rb:30
Run options: include {:focus=>true, :locations=>{"./spec/project_spec.rb"=>[30]}}

Randomized with seed 57903
F

Failures:

  1) RuboCop Project default configuration file have a period at EOL of description
    Failure/Error: expect(description).to match(/\.\z/)

     expected "Checks for flip-flops" to match /\.\z/
     Diff:
     @@ -1,2 +1,2 @@
     -/\.\z/
     +"Checks for flip-flops"

     # ./spec/project_spec.rb:34:in `block (4 levels) in <top (required)>'
     # ./spec/project_spec.rb:31:in `each'
     # ./spec/project_spec.rb:31:in `block (3 levels) in <top (required)>'

Finished in 0.07499 seconds (files took 1.61 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/project_spec.rb:30 # RuboCop Project default configuration
file have a period at EOL of description

Randomized with seed 57903
```

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch from afa3a6c to 491262e Jun 18, 2019

'not rubocop'
end
RUBY
end

This comment has been minimized.

Copy link
@koic

koic Jun 18, 2019

Member

Can you update test code using foo and bar according to example code?

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch 2 times, most recently from ef99760 to 5100c15 Jun 18, 2019

Add `MultilineWhenThen` cop
This cop check the use of the `then` keyword in multi-line when
statements.
Its name comes from `Style/MultilineIfThen` cop.
This cop is enabled by default because `Style/MultilineIfThen` cop is
enabled by default.

@okuramasafumi okuramasafumi force-pushed the okuramasafumi:multiline-when-then-cop branch from 5100c15 to 858c23d Jun 18, 2019

@bbatsov

This comment has been minimized.

Copy link
Collaborator

commented Jun 25, 2019

@okuramasafumi The specs are currently failing. You'll also have to rebase on top of master.

@okuramasafumi

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

@bbatsov Thanks, I'll fix and rebase it when I have time (hopefully next week).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.