search

leikind / wice_grid

A Rails grid plugin to create grids with sorting, pagination, and (automatically generated) filters
MIT License
537 stars 215 forks source link

Added Rubocop. #237

Closed ryanfox1985 closed 8 years ago

ryanfox1985 commented 8 years ago

Hi,

Don't worry I help you with the offenses xD. Now I'll add rspec and test.

Regards.

leikind commented 8 years ago

I don't worry. Just adding rubocop or any other similar tool does not bring anything. It's not an improvement, not a feature. It's a beginning of a certain effort. I also disagree with certain opinions that rubocop insists on.

I won't merge it.

If you like, I can add you as a contributor to the project and you can do this work in a branch of your own, if you are willing. When a certain chunk of work is completed, and it is an improvement, and it doesn't break my integration tests, then we can merge it to development.

ryanfox1985 commented 8 years ago

Hi Leikind,

well I configured in Rakefile as a opcional:

RuboCop::RakeTask.new(:rubocop) do |task| ... task.fail_on_error = false end

If you want you can follow the Ruby code style. I understand you but following the Ruby style lines is better your code will be better to understand.

Regards.

leikind commented 8 years ago

Please give me an example or examples of changes you would do thanks to Rubocop

ryanfox1985 commented 8 years ago

Hi Yuri,

Well for the moment it's so dangerous do the Rubocop refactors, you doesn't have test. I asked to you to register your library to TravisCI and I'll start to add test and get coverage.

If you launch rake before the generate the documentation launch Rspec test and Rubocop, you can take a look for the offenses.

Regards.

leikind commented 8 years ago

I have lots of capybara tests in my testbed app. I believe integration tests are what such a plugin needs.

Looking at Rubocop output I see a lot of complaints which with I agree. Such changes as adding a whitespace or replacing {} by do end are safe to perform, so I will merge it and correct code where I agree with it.

leikind commented 8 years ago

https://github.com/leikind/wice_grid_testbed

leikind commented 8 years ago

Here https://github.com/leikind/wice_grid/commit/77502b4b780cb6319622a470b470d3540dd030b7 I fixed (well, Rubocop did) those offences with which I agree. In .rubocop.yml I disabled those about which I don't care, or find simply wrong.

If I don't disable any cops, the number of offences before the fixes is 1794, after the fixes - 931. That is 863 offences fixed.

ryanfox1985 commented 8 years ago

Nice :) There are offenses no important like hash tabular, line chars.. etc. But there are important offenses, by the way you can check after.

2015-07-15 21:39 GMT+02:00 Yuri Leikind notifications@github.com:

Here 77502b4 https://github.com/leikind/wice_grid/commit/77502b4b780cb6319622a470b470d3540dd030b7 I fixed (well, Rubocop did) those offences with which I agree. In .rubocop.yml I disabled those about which I don't care, or find simply wrong.

If I don't disable any cops, the number of offences before the fixes is 1794, after the fixes - 931. That is 863 offences fixed.

— Reply to this email directly or view it on GitHub https://github.com/leikind/wice_grid/pull/237#issuecomment-121723252.