11 ms·

Ugh, no. I’ve worked in a codebase where CI would reject changes that had too much ‘code complexity’. You’d constantly have to find clever ways to split up your

• juancb 5y ago
  Did you also have the same feedback from your IDE? In other words would it have been less painful if you didn't have to suffer the long iteration times required to get feedback from some remote CI job?
  • nikita2206 5y ago
    The thing is that cyclomatic complexity for example (most popular complexity measure that linters use), it doesn’t make sense. Most of the time high cyclomatic complexity of a method is indicative of high business logic complexity… which is fine. And dogmatically saying that methods shouldn’t have that many lines and branches and that you should just come up with better abstractions doesn’t help anyone, whereas having closely related functionality concentrated in a single place, rather than synthetically exploded into N different files, well this does help.
• rhn_mk1 5y ago
  I'm sorry about your experience, but from what it seems, your problem was in your team. Any measure can be turned into a bad policy, code complexity is a red herring here.
• pydry 5y ago
  Couldnt yout just raise the threshold by tweaking the config with your PR instead? I do this all the time with other automated checkers (linters, etc.). I don't see why this should be different. If another human agrees it shouldnt be a problem.
  • sobkas 5y ago
    Because if you change that config file, people responsible for it will be added to review, and will beat you with a stick for touching it without consulting change with them?
    • pydry 5y ago
      That sounds like a people problem that will manifest in all sorts of terrible ways and not really an indictment of code complexity metrics.
• okl 5y ago
  Use the tool, don't become its slave.
• Forge36 5y ago
  I was removing unused functionality, work became held up as the code complexity was too high (never mind I'd just reduced it). I don't know what tool they were using, and they didn't share the output. I asked them to document what they found, got a mouthful on this not being their responsibility.
  • hutzlibu 5y ago
    Sounds like a place to leave as soon as possible. Unless the tool just meassured for amount of change and flagged it for review, which might make sense, as you can also mess up by removing things, you think are unused.
  • szundi 5y ago
    Just run.
  • lasereyes136 5y ago
    Sounds like a group that didn't understand the tool and just put it in place. Metrics and tools make good servants but poor managers.
• bradgessler 5y ago
  I’ve found most CI checks like this are a crock. Forgot empty parentheses for that function definition with an arity of 0? Bzzzzzzzt! Sorry! Build failed. It’s just foolish. The only reason a CI should ever fail is if it catches a defect from making it into production.
  • preseinger 5y ago
    > The only reason a CI should ever fail is if it catches a defect from making it into production. Mechanical checks for things like formatting rules, linting errors, and reasonable tools to verify code complexity, as long as they don't produce false positives, are all important to run as part of CI.
• slaymaker1907 5y ago
  Cyclomatic complexity (which I think is the most common one used) also doesn't really map to what people understand as complex. A giant switch statement with one return per case isn't nearly as complicated as two large nested if/else blocks. Nesting almost inevitably means more contexts you need to keep track of when reading code, particularly nester conditionals.
• amw-zero 5y ago
  I always had good experiences with it. I found abstractions that really did make sense, instead of treating it like a chore.