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

Run autocorrections using c5-conventions/rubocop #147

Merged
merged 2 commits into from
Jan 25, 2020

Conversation

bunnymatic
Copy link
Contributor

Problem

Shouldn't we eat our own dog food?

This repo was way out of sync with our current rubocop conventions.

Solution

Bring it up to speed.

Because we don't have rubocop in the project, this can be seen as
a one-time cleanup. We can decide later if we want it more persistent.
But for now I did the following:

  • Copy .rubocop.yml from carbonfive/c5-conventions
    curl https://raw.githubusercontent.com/carbonfive/c5-conventions/master/rubocop/rubocop.yml > .rubocop.yml
  • Locally add rubocop and rubocop-performance gems
    gem install rubocop
    gem install rubocop-performance
    
  • run it
    rubocop -a

Copy link
Contributor

@mattbrictson mattbrictson left a comment

Choose a reason for hiding this comment

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

I like it. 👍

I agree that formally adding RuboCop to the project can be a separate PR (issue #149).

Before merging this, can you open a PR to explicitly set our required Ruby version to be 2.4+? I would like to merge that 2.4 requirement prior to merging these changes. See my comment below. Thanks!

return self unless STDOUT.isatty

begin
require 'Win32/Console/ANSI' if RUBY_PLATFORM =~ /win32/
require "Win32/Console/ANSI" if RUBY_PLATFORM.match?(/win32/)
Copy link
Contributor

@mattbrictson mattbrictson Jan 23, 2020

Choose a reason for hiding this comment

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

String#match? did not exist until Ruby 2.4. Currently we don't control what versions of Ruby are allowed to use raygun, so this has a small chance of being a breaking change, if someone is using Ruby < 2.4.

Given that Ruby < 2.4 is officially EOL, I think it is fine to require Ruby 2.4+ for raygun. We can do that by adding a line to our gem spec:

gem.required_ruby_version = ">= 2.4.0"

I think that warrants a separate PR for visibility. Do you mind opening one?

@bunnymatic bunnymatic force-pushed the chores/run-rubocop-with-c5-conventions branch from 831852a to 7c0aeca Compare January 24, 2020 07:54
@bunnymatic bunnymatic mentioned this pull request Jan 24, 2020
@mattbrictson mattbrictson changed the title Run c5-conventions/rubocop on this project Run autocorrections using c5-conventions/rubocop Jan 24, 2020
Jon Rogers added 2 commits January 24, 2020 18:37
Problem
-------

Shouldn't we eat our own dog food?

This repo was way out of sync with our current rubocop conventions.

Solution
--------

Bring it up to speed.

Because we don't have `rubocop` in the project, this can be seen as
a one-time cleanup.  We can decide later if we want it more persistent.
But for now I did the following:

* Copy `.rubocop.yml` from `carbonfive/c5-conventions`
  ```bash
  curl https://raw.githubusercontent.com/carbonfive/c5-conventions/master/rubocop/rubocop.yml > .rubocop.yml
  ```
* Locally add `rubocop` and `rubocop-performance` gems
  ```bash
  gem install rubocop
  gem install rubocop-performance

  ```
* run it
  ```bash
  rubocop -a
  ```
@bunnymatic bunnymatic force-pushed the chores/run-rubocop-with-c5-conventions branch from 7c0aeca to c6fe3aa Compare January 25, 2020 02:49
@bunnymatic bunnymatic merged commit 692c8c4 into master Jan 25, 2020
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.

2 participants