Skip to content

Conversation

@dfalbel
Copy link
Contributor

@dfalbel dfalbel commented Sep 6, 2024

Add support for reticulate sessions. See posit-dev/positron#4603

@dfalbel dfalbel requested a review from lionel- September 24, 2024 17:00
#' Used by the front-end to install reticulate
#' A modal asking to install is shown before calling this.
#' @export
.ps.rpc.install_reticulate <- function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that be .ps.rpc.install()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frontend is able to set verbosity right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually shouldn't the frontend use the existing way of installing packages, i.e. r.packageInstall()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r.packageInstall is essentially just devtools::install() it doesn't allow installing arbitrary packages.
I can add .ps.rpc.install_package(pkg) if we want a more general approach.

yes, the front-end can set verbosity

@lionel-
Copy link
Contributor

lionel- commented Sep 26, 2024

My main comment is about try_lock() which should be replaced by lock() to avoid rarely occurring surprising behaviour.

@dfalbel dfalbel force-pushed the feature/reticulate-support branch from bcb8bb7 to f582e91 Compare September 26, 2024 14:04
@dfalbel
Copy link
Contributor Author

dfalbel commented Sep 26, 2024

I addressed most points in 286a24a

Also moved .ps.rpc.install_reticulate() into a more general .ps.rpc.install_packages.

Copy link
Contributor

@lionel- lionel- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

@dfalbel dfalbel merged commit 4cc60f7 into main Sep 27, 2024
@dfalbel dfalbel deleted the feature/reticulate-support branch September 27, 2024 17:10
@github-actions github-actions bot locked and limited conversation to collaborators Sep 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants