-
Notifications
You must be signed in to change notification settings - Fork 3
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
RRP instruction updates #199
Conversation
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.
Thank you for doing this!
A couple thoughts (also in my line comments):
- Do we need a separate pandoc install for wsl or just add it to the list we are already installing?
- do we want to recommend Posix line endings? I can't decide, but it probably depends on behavior that I don't quite know.
In RStudio, go to `Tools > Global Options` . | ||
Click `Code` on the left-side menu, and select the `Saving` tab. | ||
|
||
On this screen, ensure that "None" is selected from the "Line ending conversion" dropdown. |
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.
Should we say "or Posix"? I can't decide which is better default here... None seems pretty safe, but I actually don't know what will happen with new files, which could be an issue at times.
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.
It's also tricky since this isn't a project-specific setting, but a global RStudio one...
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.
It's also tricky since this isn't a project-specific setting, but a global RStudio one...
I am glad to have been kind of wrong here. It is a global setting, but one that projects can override. By default, they use the global setting, but this can be modified per project.
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.
I feel like posix is the better default overall, but I don't love that we are telling them to modify other files...
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.
What if we exclude this from setup, but add to the workshop itself in instructor notes so that this only applies to workshop files? When they open the RRP project, we can direct them to modify the project settings to use Posix. This will also give us a moment to contextualize why we are taking this step.
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.
I think that may make sense. Line endings are enough of a bugbear that they seem worth adding to explicit instruction. (We can also set this to posix in the existing project files, just to avoid the issue if we forget about 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.
We can also set this to posix in the existing project files, just to avoid the issue if we forget about it
good call
bugbear
I am wholly unfamiliar with this term, but the internet tells me it's DnD. Personally I like it because it's a tardigrade.
miscellaneously noting some backstory here: r-lib/usethis#1072 I'm don't think Posix would cause a problem necessarily? Windows code should still run on Windows, right? |
Co-authored-by: Joshua Shapiro <[email protected]>
I've updated instructions for line endings as discussed, let me know how it looks! |
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!
Closes #197
Closes #198
This PR tackles two small-ish issues:
#197
I updated installation instructions to include installing Pandoc.
apt-get
. It should not be needed for Windows proper, since this pertains to knitting via command line.#198