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 #26

Closed

Conversation

michaelachrisco
Copy link
Contributor

Closes #24

Rubocop first pass
  -  5 offenses left
@michaelachrisco michaelachrisco changed the title Fix Gem Gemfile to bundle with soda.gemspec Rubocop Jun 8, 2015
@michaelachrisco
Copy link
Contributor Author

It might be more productive to rubocop each file rather than the entire project but Ill let you guys decide. Let me know if this works out for you.

Also #25 should tell me if these changes are breaking for ruby 1.9 since I ran this rubocop on 2.2

@chrismetcalf
Copy link
Contributor

This one is going to take me a bit of time to read through. Looking now. :)

@michaelachrisco
Copy link
Contributor Author

Now that you have travis, I can break it down file by file if need be and make sure each passes for different versions of ruby. I know that some of these changes will break older versions of ruby. I also added in thoughtbots rubocop config but feel free to browse through and remove/add any suggestions.

@chrismetcalf
Copy link
Contributor

@michaelachrisco Thanks, I looked through it and the changes all look pretty reasonable. Did they pass the tests locally? I didn't see anything that'd introduce breaking changes in my code read.

Also, I am now embarrassed by my Rubies 😞

@housepage Can I get your 👀 on this too?

@michaelachrisco
Copy link
Contributor Author

Locally all the tests pass. But travis should take care of anything I am missing. 👍

@chrismetcalf
Copy link
Contributor

Woot.

@michaelachrisco
Copy link
Contributor Author

Looks like 2.2 might have some issues. I would suggest not merging until #27 is passing.

@housepage
Copy link
Contributor

@chrismetcalf Giving this a look now. @michaelachrisco Can you can get this branch back in an automergeable state please?

@housepage
Copy link
Contributor

Everything looks good to me. Did the rebase, added rubocop as a dependency, added a rake task, and added it to the travis build in #30. Thanks so much for your contribution @michaelachrisco!

@housepage housepage closed this Jun 11, 2015
@michaelachrisco michaelachrisco deleted the rubocop_best_practice branch June 13, 2015 00:19
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