troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4.05k stars 280 forks source link

Making reek smell-free #231

Closed troessner closed 9 years ago

troessner commented 10 years ago

I think we should lead by example and right now our example is failing big time with 129 total warnings reported on lib/..:)

How about we make reek absolutely smell-free?

We could also use this major overhaul to revamp stuff like how we initialize smell warnings, use the smell configuration (a little obscure IMHO) and so on....

What do you guys think?

gilles-leblanc commented 10 years ago

I think it's a wonderful idea. I know stuff I did needs to be cleaned up and I would volunteer for some of this stuff. Unless you or @mvz wants to do it in one big pull maybe this could be split up in many refactorings?

troessner commented 10 years ago

I think doing that in one big move would be impossible since I am pretty sure we need to touch every other line of code.....so in my mind, we should split this up in many refactorings.

mvz commented 10 years ago

Yes, many refactorings is definitely the way to go.

troessner commented 10 years ago

Bonus points: our utopical specs in /quality would mean something again..:)

bf4 commented 10 years ago

And then once you get smell-free, make smells break the build :) rubocop used that strategy to make sure it passed all the lints.

kevinrutherford commented 10 years ago

Hi, I think this is a good thing to aim for, but please remember something critical about Reek -- it was never intended to be "the police". The reports it makes about code were called Warnings, because they are subjective.

The original idea of Reek was that it would report all kinds of things that might be symptoms of bad design, but it could never be definitive. You can make your code report no warnings by configuring the detectors off, either for the whole project or for a single method. That's deliberately leaving the decisions of what is a "smell" in the hands of the user.

I never attempted to get Reek completely "clean", but I did create a config file that disabled the smells I don't personally care about. By all means work to get the code as well designed as possible, and use Reek as a guide along the way. But sometimes Reek can warn you about code that's actually fine, or that would be compromised too much by any attempt to "fix" the smell.

And don't forget that Reek will miss many, many types of terrible design...

Cheers,

Kevin

http://xpsurgery.com -- remote one-to-one tutoring in TDD and OO http://kevinrutherford.co.uk -- software development coaching http://refactoringinruby.info -- Refactoring in Ruby, the book

gilles-leblanc commented 10 years ago

@kevinrutherford Wise words! I feel this is often the problem with automated checks and design rules imposed in an office environment. The rules themselves do not posses intelligence or judgment. In the end only the developer does.

I do feel though that reek itself has too many warnings and that some refactorings to the existing code base would be beneficial.

troessner commented 9 years ago

And we're down to 88 warnings!.:)

Closing this, since cutting down our warnings will be more of an ongoing task in the future.