Poposal: Remove codeclimate from core


#1

Hello,

I find codeclimate rater not useful, it’s us the reviewers who decide what is the sensitive threshold of what’s too long method or line, not this tool. I’ve learned over the time to completely ignore it and I literally never go there as its output is not relevant in 10 years old Rails code base. I don’t mind anyone using it, however this happened now multiple times (last time on Friday).

I was finishing a review, everything went smooth and the result report looked like the regular “only codeclimate has problems” so I merged:

Except it was not codeclimate but katello failed tests, a good hundred of them. Yes, it was my fault, however codeclimate does not make it easier I actually see it as a struggle I need to work with.

So the Monday poll is.

  • Remove codeclimate from github PR
  • Git gut and keep the thing

0 voters


PR merged to Foreman develop broke Katello PRs
#2

Github does support blocking merges if certain checks fail. We could add Katello there, but it does mean we need them to be very stable in order not to block process.

That said, I don’t care about it so I’m neutral about it.


#3

I like the codeclimate pushing me to write better code and I am trying to make it happy. I believe the humans are more benevolent in code smells and thus I like some pedant watching over.
But you are right, that it would be nice if it shut up a bit about cases “I have just moved this freaking file, why I should fix 10 code climate issues”

I think we should AIM for “only :green_heart: goes in” state, so I would love to see codeclimate opt out for false positives as it is annoying in cases I can do nothing about, but I believe generally codeclimate is usefull.


#4

When this was introduced, we agreed it won’t be enforced. It was only helping reviewer to spot areas that could be improved. In last few years I saw people splitting the code into 2 methods just to make it shorter, without thinking about how to structure the code logically. Since contributors don’t know this is optional, they try to fix it no matter what.

Therefore I vote for disable. Sometimes it has negative impact on code quality, sometimes it causes frustration for new contributors.


#5

One more solution comes to my mind - is it possible to set this job in a way that it always pass? So people who are interested in reviewing codeclimate output would be able to do it by visiting the link. I don’t have access to theforeman org on codeclimate it seems however I think the chances are low.


#6

@lzap, it’s +9/-1, what now? :slight_smile:


#7

Well I have no admin permissions, so someone could pick this up? Looking probably at @ohadlevy or @tbrisker


#8

I’ve disabled the code climate webhook.


#9

Thanks @lzap and @tbrisker :slight_smile: