Adds min package version

[averissimo]

WIP :: parent issue: insightsengineering/nestdevs-tasks#7

Supersede:

• verdepcheck #780

🔴 What's needed before merging?

This PR depends on some upstream changes that need to be finalized/merged before being ready to review.

Change in code

• verdepcheck.yml action (see comments)
  • Remove on: push section
  • Change branch to main

PRS

• verdepcheck
  • Training verdepcheck#24
  • Fixes and improvements for strategies verdepcheck#26
• verdepcheck-action
  • Supports new strategies min_isolated and min_cohort r-verdepcheck-action#16

Changes description

• Adds minimum version for packages DESCRIPTION
• Adds Config/Need/verdepcheck section in DESCRIPTION
• Updates verdepcheck action

[shajoezhu]

hi @averissimo and @pawelru , I was wondering can we check what is the status on this PR please. Thanks

[pawelru]

thanks for the remainder @shajoezhu
I agree, if there are no blockers - let's try to merge this.

[averissimo]

PR was rebased against @main and action is currently running. If there's no blocking I'll change the status for it to be merged

[averissimo]

The reason why this was not merged before is due to {teal.slice} and {teal.data} not being on CRAN AND not being a direct dependency.

None of the strategies currently pass on {tmc} until that is resolved.

Bottom line: there's still a blocker IMO

[pawelru]

The reason why this was not merged before is due to {teal.slice} and {teal.data} not being on CRAN AND not being a direct dependency.

Are you sure it's AND? While we can definitely make packages to be available on CRAN, I don't think we want to change the dependency map - teal.slice remain as indirect dependency to tmc.
For me it's rather OR than AND but actually I think it's more about lack Remotes. This is needed especially for "max" strategy when development version of directly dependent package (e.g. teal.data) would require development version of indirectly dependent package (e.g. teal.slice). I think we need to re-open this topic again OR read below.

I have yet another thought that I am not quite sure if fully correct and I didn't have enough time to validate it.
I was thinking that verdepcheck utilises Config/Needs/verdepcheck fields recursively, that is: tmc is using DESC file of teal.data from main and that file will have a reference to teal.slice in its Config/Needs/verdepcheck. So that information is there and should be used from there

[averissimo]

What I meant is that it's blocked because of both being true. It wouldn't be a problem if

• If it was a direct dependecy, but not on CRAN
  • Only release strategy would fail
• If it's on CRAN, but not an indirect dependency
  • This *might* result in all checks failing if current @main depends on API not yet released on CRAN

I was thinking that verdepcheck utilises Config/Needs/verdepcheck fields recursively

I don't think so, but I'll test it first thing tomorrow

[pawelru]

Checked this and the answer is no.

In verdepcheck we have following default configuration:

dependencies = c(.desc_field, pkgdepends::as_pkg_dependencies(TRUE)$direct)

According to the docs:

If a character vector, then it is taken as the dependency types for direct installations, and the hard dependencies are used for the dependent packages.

I don't yet know if that's correct or not. I tried to change it to a list with direct and indirect elements to respect our custom field for indirect also and it pkgdepends crashed :( Already raised the issue - let's see how it goes.

[shajoezhu]

hi @averissimo and @pawelru , do you think we should also have this in just before tmc cran release?

[averissimo]

We could actually merge this today as AFAIK it won't have any negative impact other than failing scheduled CI

(which is already failing as it uses an old version of verdpcheck)

The CI will start to pass as soon as {teal.data} (going to be submitted today-ish) and {teal.slice} (soon) are on CRAN.

[shajoezhu]

The CI will start to pass as soon as {teal.data} (going to be submitted today-ish) and {teal.slice} (soon) are on CRAN.

Exactly! Let's do it! Thanks a lot!

[github-actions]

Unit Tests Summary

  1 files   33 suites   3s ⏱️
150 tests 150 ✅   0 💤 0 ❌
282 runs  170 ✅ 112 💤 0 ❌

Results for commit cdd6fb5.

♻️ This comment has been updated with latest results.

[m7pr]

Will the min version of packages be different if we decrease some dependencies during https://github.com/insightsengineering/teal.modules.clinical ?

[shajoezhu]

can we merge this in, and update in #981 please @m7pr

[averissimo]

@m7pr the individual versions of the dependencies are independent of each other.

In other words, they are set at a version where

• That specific version is installable (where previous ones are not)
• Contains all needed API calls

Let's say if {tibble} needed a newer version of {dplyr}, that is something that the installation procedure will resolve by itself.

We only care about the direct API used by the package, we don't map any secondary deps needs in the minimum version.

[m7pr]

@m7pr the individual versions of the dependencies are independent of each other.

I don't think if I get that but you are the boss in here : D So I guess #981 #618 is not a blocker

[pawelru]

👍