Pedagogical Review of Targets 3

[aaarcher-usgs]

To be honest, I had forgotten just how thorough and important targets-3 is! This is a great asset and very powerful training module. I noticed a few things on my run-through noted below. I'm excited to put these to the test again with the next round of onboarding!

My comments are split out by required changes "RC" and suggested changes "SC"

course-instructions.md:

xx[- [ ] SC: Add YAML and whatnot to this doc so that it's readable in html preview? Right now, it looks messy. ]xx

• [x] RC: Also, add information to the plain README that the learners should follow the "course-instructions.md" and that it can be viewed with the "preview" button in RStudio OR better within GitHub
• [x] RC: Add "fork this repository" instructions before cloning? Otherwise, adding collaborators isn't possible without DGEC license.
• [ ] RC: It is a bit disorienting to make a static-branch and then to read immediately about dynamic branching. If the "read about these types of branching methods" came before making a new branch, that would help.
• [ ] SC: Add somewhere early on that the "error" message: Ugh, the internet data transfer failed! Try again. is not a targets error, it's "an error built into the pipeline for later illustration" (so people don't spend a bunch of time trying to "fix" it when it just needs to be run again.) OR update the error message to say "try to run the pipeline again" -- Try again makes it seem like you have to "try to write the pipeline again"
• [ ] RC: The "Appliers" section still says that you're adding some files to the repository. Is that meant to change so that they're there always? Or does this rely on someone to add them manually?
• [ ] SC: The sentence near the bottom that begins "One common practice is to wrap any..." I think should go sooner when tibble is introduced. There, you could say, "we will use other tidyverse functions later, so for brevity we will load tidyverse rather than just the individual tibble package" or something like that

3_visualize/src/map_sites

• [ ] RC: Line 14 needs to have "size" replaced with "linewidth": geom_polygon(fill = "lightgrey", color = "#ffffff", linewidth = 0.25)

Random thoughts:

• Have you considered publishing these via a blog?? I would be happy to help write those in FY24, I think that would be pretty awesome

[padilla410]

Howdy @aaarcher-usgs ,

Thanks for the review!!!

Do you have an example of a repo that addresses this req change:

Add YAML and whatnot to this doc so that it's readable in html preview?

I haven't done this before

[aaarcher-usgs]

I edited the review to remove the SC about the YAML header. It's just when you look at it through RStudio that it looks weird, but it looks good as viewed on Github. There's no simple way to add the YAML to the markdown that will keep the fancy formatting, so disregard that suggestion.

I still think it would help though to have instructions on the main readme that explain how to work through the course instructions (i.e., view on Github) etc.

[jpadilla-usgs]

DRAFT: review response

Actions:

• converted to template

Suggestions that will applied to Targets 1 and 2:

• conversion to a template
• "getting started" guidelines