Make cyclocomp_linter() optional

[IndrajeetPatil]

As Michael mentioned in #2539:

there's not anything specific in the style guide about meeting certain complexity requirements & that linter is quite heavy, we could make it optional.

[IndrajeetPatil]

Thoughts I jotted down after having worked on #2555:

Carving out all exceptions in unit tests is really making me iffy about whether this change is desirable. It breaks the beautiful isotropy (read uniformity) we have across linters where we don't need to treat any linter in a special manner.

Additionally, this is a breaking change, especially in the context of CI: everyone who has been using this linter is going to be forced to update the CI configs to additionally download {cyclocomp}.

Wouldn't porting over the {cyclocomp} code (MIT licence) in {lintr} be a better option?
Since {cyclocomp} is already in a stable state, the maintenance burden also wouldn't be too onerous.

WDYT, @MichaelChirico, @AshesITR?

Of course, the other easy solution is to maintain the status quo.

[MichaelChirico]

Carving out all exceptions in unit tests is really making me iffy about whether this change is desirable. It breaks the beautiful isotropy (read uniformity) we have across linters where we don't need to treat any linter in a special manner.

I don't follow this. AIUI, the test carve-outs are mainly because we wrote tests needing a default linter, and happened to pick cyclocomp. We could as well re-write the tests using a different default linter, right (no need to do that for #2555)?

If the idea is no linter should require add-on packages, I don't agree with that. #2149 is a great example -- we can offer a big enhancement within todo_comment_linter() by doing some API pings, but shouldn't require every {lintr} user to be equipped for that opt-in enhancement.

I worry there's too much status quo bias here. If we do make cyclocomp_linter() optional, in 3-5 years (say) it will be just like #2149 -- an opt-in linter offering nice functionality for those users that really want it.

Additionally, this is a breaking change, especially in the context of CI: everyone who has been using this linter is going to be forced to update the CI configs to additionally download {cyclocomp}.

Looking around, I see two types of reference to cyclocomp_linter() in .lintr configs:

https://github.com/search?q=path%3A.lintr%20%2Fcyclocomp_linter%2F&type=code

1. Turning it off (cyclocomp_linter = NULL), ~250 hits
2. Weakening it (cyclocomp_linter(N > 15)), ~150 hits

I think those 150 usages could be affected, I guess it will be a CI error (assuming they have a lint CI -- any such downstream where {lintr} is run locally will not be affected, as {cyclocomp} will remain installed for a long time).

Looking at the first 10 hits for cyclocomp[(], I see half with a *lint_* call somewhere in their .github/workflows. So possibly we could introduce this change with some more ease, we'd have to think of how.

In the longer term, we get that users really wanting cyclocomp_linter() will have one extra line in their CI for installing it to enhance their lint experience.

Wouldn't porting over the {cyclocomp} code (MIT licence) in {lintr} be a better option?

Not a fan of this approach where I've seen it -- worse than status quo of just depending on {cyclocomp} IMO.

---

NB: the above is just for discussion. Not dead-set on removing cyclocomp_linter() from defaults.

[IndrajeetPatil]

Okay, thanks for providing some quantitative context to my misgivings!

If the idea is no linter should require add-on packages, I don't agree with that.

I think this was the crux of my worry, and I concur that, in the long run, maybe this is something that we wish to do to gain some valuable functionality.

As for the CI issue, looking at those numbers, I don't think we need to worry too much about breaking CI for many packages. The CI failures will prompt an immediate action, and so it's not a change that will pass below the radar, which would have been more problematic.