2 cores in CRAN checks

[TysonStanley]

With our latest CRAN submission, Uwe noted:

Check: examples, Result: NOTE
   Examples with CPU time > 2.5 times elapsed time
                 user system elapsed ratio
   groupingsets 4.435  0.082   0.903 5.002

suggests you are still using more than 2 cores.

He recommended to have this fixed for the next submission (likely for us to be a full release at that point).

[aitap]

On master we have (#7632): \dontshow{.dt_threads <- setDTthreads(2)} ... \dontshow{setDTthreads(.dt_threads); rm(.dt_threads)} Somehow it never landed on the patch branch. Is the lesson here to be even more proactive about cherry-picking CRAN-related fixes into the patch branch?

[ben-schwen]

Somehow it never landed on the patch branch. Is the lesson here to be
even more proactive about cherry-picking CRAN-related fixes into the
patch branch?

Yes. I think thats absolutely our learning that we have to be more careful when not doing full releases. Moreover, we should probably think about a process how to fix a certain point of master for full releases (to not include last minute additions in a full release).

[aitap]

Maybe we need a new label, say, next-release for PRs that need to go into the release branch? I couldn't cherry-pick #7587 into the patch branch in the middle of the 1.18.2 release process and later forgot about it because that release was accepted anyway. We could set the label when submitting the PR and remove it after cherry-picking (or a successful release). (There's all kinds of possible automation, e.g., creating a PR into the patch branch when a marked PR is merged into master, but let's not go there yet.)

There's an annoying functionality overlap with milestones, but it's not always obvious whether a PR is going to be needed in 1.x+1.0 or 1.x.y+1.

Moreover, we should probably think about a process how to fix a certain point of master for full releases (to not include last minute additions in a full release).

How long would you suggest between cutting a release branch from master and releasing? SQLite developers spend approximately a week "pencils-down", but they release straight from trunk. The disconnect between master and the release branch probably shouldn't become too long, to avoid duplicating the testing effort and the need to merge fixes twice.

[MichaelChirico]

This is the sort of thing LLM can do pretty well, e.g. prompting:

Compare the 1.18.6 patch branch and all the other changes on `master` since 1.18.0. Is there anything else patch-worthy that was omitted? We aim to mirror bug fixes on both master and the patch release, reserving new feature / deprecations for only "major" (1.X.0) releases, while bug fixes can be shipped more greedily.

eventually returns the following (edited for brevity). We could of course refine the prompt to better decide which changes are meant for a patch vs. full release. Basically this is a good second set of eyes to use before hitting 'upload'.

---

• Reset isReal test per-column to fix forder() with na.last=TRUE #7877, fixing forder incorrect for na.last=TRUE and double column, descending character column #7875
• f3b166b, fixing fread() allergic to \x1a bytes #7407
• copy(): only overwrite the list when necessary, consider the attributes #7820, fixing Storing xgboost models in list columns fails #7456
• prohibit duplicate key columns #7760, fixing keys are wrong/don't update if column names aren't unique #4888, Inconsistent behavior in keyed/unkeyed joins against duplicate columns #4891
• Don't access rows of empty table #7750, fixing VECTOR_ELT issues with empty data.tables with R 4.6. #7749
• fread no longer skips on skip=0 #7484, fixing fread omitting rows without warning #7463
• fread: preserve literal "NA" header names #7686, fixing CSV header with column name "NA" is incorrectly parsed by fread #5124
• between supports Date/IDate with missing bounds #7751, fixing Inconsistent between() behavior on dates due to NAbounds argument #7281
• Preserve names in as.IDate() and as.ITime()  #7800 & Fix for as.IDate() to retain names even before some as.Date() calls did so in R (4.1.2) #7850, fixing as.IDate() drops names #7252
• Support for fread() of file:// URI with spaces #7551, fixing tests.Rraw 1378.2, 1378.3 fail when the .Rcheck directory path contains spaces #7550
• Fix col.names="none" suppressing all output when row.names=FALSE  #7804, fixing print col.names="none" suppresses print completely #7735
• Fix example(local=TRUE) for cedta (again) #7855, re-fixing Strange errors when running examples with local=TRUE argument #2972
• tryCatch for nchar() on old R #7848

[ben-schwen]

How long would you suggest between cutting a release branch from master and releasing? SQLite developers spend approximately a week "pencils-down", but they release straight from trunk. The disconnect between master and the release branch probably shouldn't become too long, to avoid duplicating the testing effort and the need to merge fixes twice.

From top of my mind I thought about 2 weeks. But maybe thats a thing we can add a deeper data analysis from when issues pop at our tracker from the time they were merged.

[TysonStanley]

Is this a formal process we want included in the open "patch" issue where we post a list of potential changes to include and have input from the devs or should we generally be more inclusive of these changes that are found (eg by an LLM)? Previously, I've been quite exclusive (focused only on the issues required for passing the CRAN checks), knowing some changes can trigger revdep issues that may not be resolved before needing to submit the patch.

[jangorecki]

With a patch release, unless something was a regression, I would go with CRAN-required only. If something was a bug before, can still wait and be a bug till next major release.

[MichaelChirico]

I agree it makes sense to keep patches minimal. I would add "fixes to bugs introduced by the latest release", so if feature X is part of 1.Y.0, and it's broken, we can include fixes to it.

Something like #7877, where the bug in the release is leading to silently incorrect results, is also critical to fix ASAP.

[TysonStanley]

Since this specific issue is resolved, we can close this one. Next patch release, I will run a check to see if any other bugs are needed to go to CRAN and will report those before shipping.