-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rstudio: clean up and refactor #357034
rstudio: clean up and refactor #357034
Conversation
1064909
to
1da1c18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
4f64168
to
0e8c1f9
Compare
I see that the rstudio server test is failing. I'm assuming it's not supposed to. Will check it out. |
Turns out rstudioWrapper expected the empty |
Rebased on master. Is everything that worked before this PR still working? If everything is OK, is there anything stopping us from merging? |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/2175 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested fully functional, previous issue of the visual editor being broken is resolved. LGTM.
Okay, I'll try to merge it in a day if nobody objects to it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Visual editor is finally working. I used to have problems with the built-in debugger (it didn't show up in the environment browser), which is also fixed in this PR. LGTM!
PS: compilation needs more RAM than previously. Restricting the number of cores helps.
Next step will be #362637, though we need to wait for electron to get cached. |
The first 2 commits should not cause any rebuilds.
The 3rd commit should only cause
rstudio-server
to rebuild. (Since it now has empty arrays asdesktopItems
andqtWrapperArgs
instead of them being undeclared)Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.