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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rubocop fixes #196

Merged
merged 1 commit into from Jul 12, 2017

Conversation

Projects
None yet
3 participants
@perlun
Contributor

perlun commented Jul 12, 2017

This makes the 馃懏 be happy.

A lot of this is just:

  • rubocop -a (auto-fix)
  • rubocop --auto-gen (generate todo config)

But, the good part about this is that we can now run Rubocop on each Travis build, to ensure that new code follows the Rubocop rules as defined in .rubocop.yml

@perlun perlun self-assigned this Jul 12, 2017

@perlun perlun requested review from junaruga and scepticulous Jul 12, 2017

Per Lundberg
Rubocop fixes
This makes the 馃懏 be happy.

A lot of this is just:

- rubocop -a (auto-fix)
- rubocop --auto-gen (generate todo config)

But, the good part about this is that we can now run Rubocop on each Travis build, to ensure that _new code_ follows the Rubocop rules as defined in .rubocop.yml
@scepticulous

Great job! I was about to ask if we already have a decision on style because of the code climate complains. This really enables us to have a solid base.
Thanks.

@junaruga

This comment has been minimized.

Member

junaruga commented Jul 12, 2017

Great job!
We will items in .rubocop_todo.yml one by one in the future, right?

I was about to ask if we already have a decision on style because of the code climate complains.

In my opinion, if possible, we should fix each code climate complains for each PR.
Because code climate complains only for modified lines of PR.

@junaruga

This comment has been minimized.

Member

junaruga commented Jul 12, 2017

In my opinion, if possible, we should fix each code climate complains for each PR.
Because code climate complains only for modified lines of PR.

The code climate may check only modified files. Not lines.

@perlun

This comment has been minimized.

Contributor

perlun commented Jul 12, 2017

Thanks to you both!

@scepticulous

Great job! I was about to ask if we already have a decision on style because of the code climate complains. This really enables us to have a solid base.

Well, we don't really have a decision other than using the defaults as provided by Rubocop. I think most of these make sense, but there are cases where I typically deviate (you saw it mentioned in the PR in the .rubocop.yml change).

@junaruga

We will items in .rubocop_todo.yml one by one in the future, right?

Yes, that'd be the idea. If people have spare time and want to work on cleaning these issues up, they are very welcome to do so.

With this change in place, the Rubocop rules will run:

  • On Travis
  • On CodeClimate

That's of course a bit silly, but we can live with it for now. (It seems the codeclimate settings use a different version of Rubocop - if someone knows how to fix that, please do. OTOH, we could disable Rubocop on codeclimate since we already run it on Travis anyway.)

@perlun perlun merged commit bfa7e9d into master Jul 12, 2017

2 of 3 checks passed

codeclimate 6 issues to fix
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@perlun perlun deleted the fix/rubocop branch Jul 12, 2017

toshimaru added a commit to toshimaru/rack-test that referenced this pull request Jul 13, 2017

Rubocop fixes (rack-test#196)
This makes the 馃懏 be happy.

A lot of this is just:

- rubocop -a (auto-fix)
- rubocop --auto-gen (generate todo config)

But, the good part about this is that we can now run Rubocop on each Travis build, to ensure that _new code_ follows the Rubocop rules as defined in .rubocop.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment