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

Rubocop fixes #196

Merged
merged 1 commit into from
Jul 12, 2017
Merged

Rubocop fixes #196

merged 1 commit into from
Jul 12, 2017

Conversation

perlun
Copy link
Contributor

@perlun 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 dennissivia July 12, 2017 06:14
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
Copy link

@dennissivia dennissivia left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
@perlun perlun deleted the fix/rubocop branch July 12, 2017 12:32
alex-damian-negru pushed a commit to alex-damian-negru/rack-test that referenced this pull request Apr 5, 2021
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants