Make cyclocomp_linter() optional

[IndrajeetPatil]

Closes #2554

Creating a draft PR. Not sure in which release we would like to include this change.

TODO:

• update NEWS

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.15%. Comparing base (d22e6ae) to head (1a07a08).
Report is 16 commits behind head on main.

❗ Current head 1a07a08 differs from pull request most recent head 2d672da. Consider uploading reports for the commit 2d672da to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2555      +/-   ##
==========================================
- Coverage   98.18%   98.15%   -0.03%     
==========================================
  Files         125      125              
  Lines        5715     5737      +22     
==========================================
+ Hits         5611     5631      +20     
- Misses        104      106       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MichaelChirico]

LGTM, I'm assuming this is ready to merge?

[IndrajeetPatil]

LGTM, I'm assuming this is ready to merge?

Yes!

[MichaelChirico]

R CMD check throws a NOTE:

* checking Rd cross-references ... NOTE
Package unavailable to check Rd xrefs: ‘cyclocomp’

Should we change that link (it won't cause any CRAN issue since {cyclocomp} will be available)? I lean towards "not an issue".

[IndrajeetPatil]

The note in rcmdcheck should be fixed.

But the failing test continues to befuddle me. Can have a look tomorrow.

[MichaelChirico]

The note in rcmdcheck should be fixed.

I'm really not sure -- wanted to flag for vis. I especially have in mind that base R itself throws the same "missing cross-reference" NOTEs a fair amount, e.g. in ?cor.test:

https://github.com/r-devel/r-svn/blob/b5582bb6dc31e988de5c33ebb28932901c2206ae/src/library/stats/man/cor.test.Rd#L131-L140

We also link to Suggests packages elsewhere, e.g.:

lintr/man/expect_comparison_linter.Rd

Lines 9 to 15 in 79d1059

	\description{
	\code{\link[testthat:comparison-expectations]{testthat::expect_gt()}}, \code{\link[testthat:comparison-expectations]{testthat::expect_gte()}}, \code{\link[testthat:comparison-expectations]{testthat::expect_lt()}},
	\code{\link[testthat:comparison-expectations]{testthat::expect_lte()}}, and \code{\link[testthat:equality-expectations]{testthat::expect_equal()}} exist specifically
	for testing comparisons between two objects. \code{\link[testthat:logical-expectations]{testthat::expect_true()}} can
	also be used for such tests, but it is better to use the tailored function
	instead.
	}

Do you have a proposed workaround in mind? Maybe there's a good alternative I haven't hit on yet.

[MichaelChirico]

Should we make this function simpler & remove the cyclocomp reference?

lintr/tests/testthat/default_linter_testcode.R

Line 18 in 79d1059

# cyclocomp

Ditto here:

lintr/tests/testthat/dummy_projects/project/default_linter_testcode.R

Line 13 in 79d1059

# cyclocomp

[IndrajeetPatil]

We also link to Suggests packages elsewhere, e.g.:

This is because we are cheating a bit. We don't treat all suggested dependencies equally while running R CMD check in "no Suggests" mode:

lintr/.github/workflows/R-CMD-check-hard.yaml

Lines 55 to 60 in 79d1059

	any::rcmdcheck
	any::testthat
	any::knitr
	any::rmarkdown
	any::patrick
	any::withr

If we were to also add {cyclocomp} here, we can get rid of that NOTE, but then our CI will never treat this dependency as truly a suggested dependency.

[IndrajeetPatil]

Should we make this function simpler & remove the cyclocomp reference?

Yes, we should remove those references.

I don't have a good sense of how much work making that function simpler will be, but I can have a look in the evening, after work, unless you get to it before me.