Get Package info

[AlexAxthelm]

This PR is a checkpoint PR, and should be considered as part of a larger work in progress, mostly containing the get_package_info and related functions.

I'm opening this as a targeted PR, so that this bit can be evaluated in isolation, since the tests for this section can get kind of gnarly.

Requests to reviewers:

• Ensure that the information emitted in the package info is what we want (not missing anything)
• Help check that the tests are actually testing what I think they are

Note that a lot of the "weirdness" in the tests (making use of a lot of expect_match vs expect_identical, or the heavy use of for example) is related to either having the tests run on the CI machines, or run on Windows, or because I don't actually want to break .liPaths() on my local machine 😆

[codecov]

Codecov Report

Attention: Patch coverage is 93.47826% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 86.41%. Comparing base (ba9b914) to head (d94cc6a).

Files	Patch %	Lines
R/export_manifest.R	0.00%	6 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           feat/1-export-manifests       #6       +/-   ##
============================================================
+ Coverage                    76.05%   86.41%   +10.36%     
============================================================
  Files                            2        4        +2     
  Lines                           71      162       +91     
============================================================
+ Hits                            54      140       +86     
- Misses                          17       22        +5     

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

[cjyetman]

I just reviewed get_individual_package_info() as suggested, but I'll try to review the rest of it later today.

[AlexAxthelm]

@cjyetman, @jdhoffa Made a number of changes since last review, but I think the big thing that's still missing on this block is the git info for local/dev packages, and I haven't written any of that yet, so I'd like to get this merged into the bigger tracking branch.

structural changes since last review:

• get_package_info takes an arbitrarily nested list/vector/(anything that can go into vapply) and spits out a nested list.
  • please look at the structures in the tests. there is a little weirdness if you're passing in an unnamed list, but I think the defaults are sensible.
• That's useful since get_individual_package_info changed the output structure, and all the useful stuff is now top-level keys
• That function also added a big if/else block dependent on if the package was loaded with pkgload (devtools), but with this, we'll have a lot more power diagnosing systems that are in local development on restricted/airgapped systems (@jacobvjk this might be relevant for the multiloanbook repo)
• Added a few dependencies. mostly suggests.

[jdhoffa]

NIT: Should we be more specific about what we mean by dev_version?

Here it is defined as something different from what is idiosyncratic in R.
In typical R packages, the concept of "dev version" is indicated by some large component in the DESCRIPTION SemVer (e.g. X.Y.Z.9000) (regardless of if it is loaded using devtools::load_all() or installed from GH using e.g. pak::pak).

Here, however, dev_version is defined as TRUE if something was loaded into the environment using devtools::load_all (see reprex).

Should we rename the flag as loaded_version or something similar to distinguish it?

(I don't mean to start a whole big discussion on what "dev version" should mean, just flagging that it may be confusing to users, or even to us down the road, as to what we are referring to when we use that term)

devtools::load_all("/Users/jdhoffa/github/pacta.workflow.utils")
#> ℹ Loading pacta.workflow.utils

out <- pacta.workflow.utils:::get_package_info(
  packagelist = c(
    "pacta.workflow.utils",
    "r2dii.data",
    "dplyr"
  )
)
#> Warning in get_individual_package_info(x): Identifying development packages may
#> not be accurate.
#> Warning in get_individual_package_info(x): Multiple installations of package
#> found.

#> Warning in get_individual_package_info(x): Multiple installations of package
#> found.

out$pacta.workflow.utils$version
#> [1] "DEV 0.0.0.9002"
out$pacta.workflow.utils$dev_version
#> [1] TRUE

out$r2dii.data$version
#> [1] "0.4.1.9000"
out$r2dii.data$dev_version
#> [1] FALSE

out$dplyr$version
#> [1] "1.1.4"
out$dplyr$dev_version
#> [1] FALSE

^{Created on 2024-03-26 with reprex v2.1.0}

[AlexAxthelm]

@jdhoffa if we renamed it to something like loaded_with_pkgload would that be better? It would be more accurate. local_dev might also work, but still feels like it's missing something.

I'd stay away from loaded_version, since that might be exactly the same version number as what's in the installed-dev (vs the local working dev version). For example, I can install a package from GitHub, change a few files (but not DESCRIPTION), and load_all and the version number would be the same (this is the main reason why I think version-bumping on every commit to main is useful)

[jdhoffa]

Yeah that's exactly my point actually!
I agree that the case it is trying to distinguish between is very useful, just being a bit nitpicky about the name.

I think loaded_with_pkgload sounds reasonable?

[AlexAxthelm]

@jdhoffa easy change. done. I think I like this clarity better.

[jdhoffa]

That was my only comment, this looks absolutely awesome man seriously!
Thank you

Leave it to @cjyetman who may have more useful feedback as he engages with the manifest outputs more frequently.

[cjyetman]

$R.version$nickname
[1] "Eye Holes"

🤣

[jdhoffa]

LOL

[cjyetman]

seems like this test name/message is out of sync with reality