Skip to content
This repository has been archived by the owner on Nov 30, 2023. It is now read-only.

Update R Docker image and vscode-r recommended settings #732

Merged
merged 2 commits into from
Feb 23, 2021
Merged

Update R Docker image and vscode-r recommended settings #732

merged 2 commits into from
Feb 23, 2021

Conversation

eitsupi
Copy link
Contributor

@eitsupi eitsupi commented Jan 25, 2021

This pull request updates R to latest version and add vscode-R recommended settings.

@ghost
Copy link

ghost commented Jan 25, 2021

CLA assistant check
All CLA requirements met.

@eitsupi
Copy link
Contributor Author

eitsupi commented Jan 26, 2021

@Chuxel I apologize for including the fix of username acaba59 in this PR.
You've changed username to docker in e2480ab, but I think vscode is more better username because this is other container's default username.
So, I have changed the username to vscode again c1548b4.

@2percentsilk
Copy link
Member

@kmehant as the R definition maintainer, can I get your eyes on this? 👀

@eitsupi
Copy link
Contributor Author

eitsupi commented Feb 20, 2021

@2percentsilk @Chuxel @kmehant If this review is going to take a long time, would it be better to break the PR into base image update and the others?

@kmehant
Copy link
Contributor

kmehant commented Feb 23, 2021

@eitsupi Looks good to me! However, I request a review from @Chuxel.
It would be great if you could squash commits wherever possible.

@2percentsilk Thanks for tagging me, I apologize for the late response.

eitsupi and others added 2 commits February 23, 2021 21:22
- Change base image from rocker/r-apt to rocker/r-ver
- Add vscode R LSP Client extention
- Add R packages, languageserver and devtools
- Add radian (R console, recommend by vscode-R)
- Add the necessary apt packages
- Remove libzip-dev, not in use
- Add vscode-R recommended settings
@Chuxel
Copy link
Member

Chuxel commented Feb 23, 2021

LGTM! The new extension is low volume which gave me pause, but I see its maintained by the same maintainers as the existing extension. So from that perspective it's probably ok.

Made one fix to move the remoteUser back to vscode given this is what the dockerfile does.

@Chuxel Chuxel merged commit 4bd3168 into microsoft:master Feb 23, 2021
@Chuxel
Copy link
Member

Chuxel commented Feb 23, 2021

@eitsupi @kmehant Actually, one thing I just noticed - the # [Choice] comment in the Dockerfile also can drive VS Code UX, but it has to be a concrete list.

At the moment I've removed the comment since I'm not sure if there's higher level tags like "4" and the lower level versions are going to be a bit of a maintenance issue.

@eitsupi eitsupi deleted the update-R-container branch February 23, 2021 23:33
@eitsupi
Copy link
Contributor Author

eitsupi commented Feb 23, 2021

@Chuxel Thank you for merging and the comment fix.

There is no higher level tags like "4", and minor versions like "4.0.4" will increase every few months.
So if there is no problem with the current state, I think it's fine the current state which only removed the # [Choice] comment.

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.

4 participants