Add GHA to copy completed notebooks #132
Conversation
There was a problem hiding this comment.
My main comment here is that if you already checked out both repos, why use curl? My suggested solution may not be quite what you want, but it should be along the right lines, and could simplify things.
I also had a naming suggestion because main is so overloaded.
Another simplifying thought is that maybe just a make an array of modules and loop through it with something like
for module in $modules; do
cp training-modules/$module/[0-9]*.html website/completed-notebooks/$module/.
done(I added some specificity back by starting with a number)
| # the url for for this tag of the training-modules repo where files will be downloaded from | ||
| base_url=https://raw.githubusercontent.com/AlexsLemonade/training-modules/${{ github.event.inputs.training-modules-tag }} |
There was a problem hiding this comment.
Why are we using a URL here rather than just copying the files we already have checked out?
| # note that at least one notebook is only .html, NOT .nb.html, so we'll just use .html | ||
| [[ $path != *.html ]] && echo "'$path' is not an .html file." && exit 1 |
There was a problem hiding this comment.
Just noting that you used *html for the search, not *.html; it would probably be better to be consistent.
There was a problem hiding this comment.
Now using [0-9]*.html throughout for copying, and we no longer have searching, because the point is to use both repos if we have both repos!
🤦♀️ is my answer I think...! |
- Call directory website, not main - Bump PR action to @5, remove safe directory creation - Stop curling, and start copying, because _of course_ - Add bulk RNAseq - Fancier PR comment - Use case statement to simplify the code while still only getting a given workshop's files
|
After my first round of excitingly using a different strategy from exercise notebooks, and then almost impressively forgetting I was taking that strategy half-way through writing 🤪, here is round 2! Again, this was tested in a separate private repo of mine to save us all from spam, hence the single commit (which to be clear I just taught a workshop about how this was a bad practice, but here we are!). I directly incorporated suggestions myself, including Let me know what you think now, but again, no rush on re-reviewing this today if that's not good for your schedule! |
|
|
||
| - name: Configure git | ||
| run: | | ||
| # Configure git for just the `website` repo local credentials |
There was a problem hiding this comment.
I had no problem with the global settings. It seems safer than changing directories.
| target_path=website/completed-notebooks/ | ||
|
|
||
| # Copy notebooks over depending on which training module is being taught | ||
| case "${{ github.event.inputs.training-module }}" in |
There was a problem hiding this comment.
I had forgotten how ugly case statements in bash are, but I guess it kind of makes sense here.
But I did like my for loop idea to have only one cp statement.
case "${{ github.event.inputs.training-module }}" in
"Introduction to R and Tidyverse")
modules=("intro-to-R-tidyverse")
;;
"Introduction to single-cell RNA-seq")
modules=("intro-to-R-tidyverse" "scRNA-seq")
;;
and so on...
followed by (syntax correct this time)
for module in ${modules[@]}
do
cp training-modules/${module}/[0-9]*.html ${target_path}
done
There was a problem hiding this comment.
Ahh I see, I like this!
I had thought about this at one point, but got nervous about finding all the links in all the places since it's spread across several repos. As long as we don't delete |
Yeah, I guess I was thinking that this repo might be the one to start with? Nothing should link back to it that I am aware of... |
…cp line. Tested locally as script and syntax is good.
|
@jashapiro was there anything else here? I think this was pretty much set? |
Co-authored-by: Joshua Shapiro <josh.shapiro@ccdatalab.org>
Closes #111
This PR adds a github action that will copy completed notebooks from a given tag of
training-modulesinto this template repository (presumably after someone has copied the template!). All repos involved are public, so no special security things here, which is important since we might expect folks running external workshops to use this action.I developed this workflow in one of my personal repositories so I could spam myself to my heart's content, hence only one commit here. I confirmed it worked for any of the three workshops, but we should still do a final.final round of testing here as well. I figure we may want to get any conceptual reviews implemented first though, so this action currently only runs manually. Once we're confident that this is how the GHA should actually look, I can add a PR target so it will run here.
I made a variety of choices along the way which we can discuss during review if there are strongly differing opinions. Here's what I did!
training-modulesrepo tag of interest, which defaults tomaster.training-modulesand this one. You have to be a little careful with paths here, but it seems to work ok! I opted for this approach to find the files to copy rather than maintaining something analogous to anexercise_listlike we do for the exercise copying action.main, which seemed like a good generic name for the repo we are PRing into (it won't always betraining-specific-template, since this action will generally be run in a differently-named repo created from this template).--globalflags for all thegit configsettings.cd mainand then set local username/email if that is preferred.completed-notebooks/.nb.htmlfiles, but we have one.html(not a notebook) living in intro scRNA-seq, so I went more permissive to just.html.One key question for reviewers: Would we rather see the bash code pulled out into it's own script to live in
scripts/? I was on the fence at the beginning, but now I'm trending towards "yes".