New function to expose more info about an installed package #5365
Conversation
|
I'm going to bring caching back with a bit of modification. Stay tuned. |
jennybc
force-pushed
the
package-version-info
branch
from
049f58b to
f986a4d
Compare
Why? * Sometimes you want to get info about a package version without necessarily challenging the user to install/upgrade. * If you do need to nudge the user, this extra info allows us to build a better message. I also removed the caching, because the extra info makes that increasingly awkward.
jennybc
force-pushed
the
package-version-info
branch
from
f986a4d to
604f2fe
Compare
extensions/positron-r/src/session.ts
Outdated
| pkgInst = await this.packageVersion(pkgName, minimumVersion); | ||
| compatible = pkgInst?.compatible ?? false; | ||
| return compatible; |
There was a problem hiding this comment.
This is a bit new: previously we always assumed that our attempt to install/update the package was successful. Now we check that specifically, cache new version info, and return the result (hopefully true).
|
OK @juliasilge this is done now, complete with restored and upgraded caching. Additional work that is not shown here: I temporarily added lots of logging to verify that the cache was being used/updated as expected and I was satisfied with the results. |
There was a problem hiding this comment.
I think something here isn't quite working right yet.
With our current release build, if I try to create a new project using renv but I have renv 1.0.8 installed, I see this:
With a dev build in the same situation, the new project opens up but there is no message or prompt to install renv.
Maybe it's just that ark needs to be bumped in this PR?
| @@ -33,6 +35,7 @@ interface EnvVar { | |||
| // locale to also be present here, such as LC_CTYPE or LC_TIME. These can vary by OS, so this | |||
| // interface doesn't attempt to enumerate them. | |||
| interface Locale { | |||
| // eslint-disable-next-line @typescript-eslint/naming-convention | |||
|
I have already rebased and force pushed this PR branch to include the ark version bump I did earlier today. Do you need to refetch this branch? Any chance you've got a local ark build that's getting used in the dev build instead of the 0.1.154 release from this afternoon? |
|
I just re-did the QA step described above and I'm still getting the desired pop-up, with locally built ark and after doing 
|
|
I think it's possible what we need is step 4 here, like this commit. |
|
🤔 step 4 has happened, in Here's the ark requirement in positron/extensions/positron-r/package.json Line 673 in d0e363d It does feel like an ark mismatch. Does |
|
I made one more change, but it would not explain what you're seeing (a complete lack of prompt). |
Addresses #1957
Requires posit-dev/ark#625
What's the payoff?
QA Notes
To experience that
checkInstalled()still works and to see the user-facing message in the absence of a minimum version requirement, here's one idea:remove.packages("styler")You should see this:
To experience that
checkInstalled()still works and to see the user-facing message in the presence of a minimum version requirement, here's one idea:Install renv at a version below positron-r's minimum version, which is currently 1.0.9. I would do this with
pak::pak("[email protected]").Walk through File > New Project > R Project and click the box to use renv:
As the new project is being stood up, you should see this: