Skip to content
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

Allow overview pages #132

Merged
merged 20 commits into from
Aug 28, 2023
Merged

Allow overview pages #132

merged 20 commits into from
Aug 28, 2023

Conversation

zkamvar
Copy link
Contributor

@zkamvar zkamvar commented Aug 8, 2023

This will allow workshop overview pages to be processed even if they do not have any episodes.
There is a new $overview field for the Lesson object that is set to TRUE if we detect that the lesson is an overview.

For lessons built with The Workbench, this requires the overview: true YAML item. For lessons built with Jekyll, this inelegantly checks that the folder name ends with -workshop (which is violated by the LibraryCarpentry lesson using -overview, but it's moot since that lesson has episodes).

This addresses part of carpentries/workbench#65

To review this PR, start by reviewing the changes in the Lesson.R file (note: the Lesson$new() is defined by the initialize method. This is where everything flows from, which looks like this where new functions are labelled as [NEW]:

flowchart TB
   L["Lesson$new()"]
   jeky["read_jekyll_episodes()"]
   sand["read_sandpaper_lesson() [NEW]"]
   mark["read_markdown_files()"]
   sort["sort_files_by_cfg() [NEW]"]
   L -->|"jekyll=TRUE"| jeky
   L -->|"jekyll=FALSE"| sand
   jeky --> mark
   sand --> mark
   mark --> sort
Loading

I've split out the processing for the sandpaper lessons into its own function because the initializer was a little too complex.

I've also split out the functionality to sort files by the order of the config in read_markdown_files because it was also pretty complext to read initially.

NOTE: I am bumping the minor version number since this is a new feature.

Testing

You can test this version of {pegboard} by using the pull request helpers from the {usethis} package inside your local clone of {pegboard}

library("usethis")
library("devtools")
pr_fetch(132)
load_all()
# get_lesson() will download a lesson to a temporary directory and then read it in with `Lesson$new()`.
get_lesson("datacarpentry/genomics-workshop")  

When you are finished, run:

pr_finish()

If everything works, you should see this output:

<Lesson>
  Public:
    blocks: function (type = NULL, level = 0, path = FALSE) 
    built: NULL
    challenges: function (path = FALSE, graph = FALSE, recurse = TRUE) 
    clone: function (deep = FALSE) 
    episodes: NULL
    extra: NULL
    files: active binding
    get: function (element = NULL, collection = "episodes") 
    handout: function (path = NULL, solution = FALSE) 
    initialize: function (path = ".", rmd = FALSE, jekyll = TRUE, ...) 
    isolate_blocks: function () 
    load_built: function () 
    n_problems: active binding
    overview: TRUE
    path: /tmp/RtmpetwTRo/datacarpentry--genomics-workshop
    reset: function () 
    rmd: FALSE
    sandpaper: FALSE
    show_problems: active binding
    solutions: function (path = FALSE) 
    summary: function (collection = "episodes") 
    thin: function (verbose = TRUE) 
    validate_divs: function () 
    validate_headings: function (verbose = TRUE) 
    validate_links: function () 
  Private:
    deep_clone: function (name, value) 

Created on 2023-08-08 with reprex v2.0.2

This will fix #118

@zkamvar zkamvar requested a review from a team August 8, 2023 17:24
Copy link
Contributor

@klbarnes20 klbarnes20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this and everything seems to work. I have added a few minor non-blocking suggestions.

R/read_jekyll_episodes.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
zkamvar and others added 6 commits August 28, 2023 10:53
This makes the statement more clear (thanks to @klbarnes20 for the
suggestion)
Co-authored-by: Kelly Barnes <64787509+klbarnes20@users.noreply.github.com>
I had initially tried to design this to account for the librarycarpentry
overview lesson, which had episodes, but I realized that it could just
be coded as a standard lesson without necessarily gaining overview
status and that if they wanted to switch over, it would be a matter of
moving the episode subfolders to the top directory and adding the
`overview: true` yaml item to the config.
I've added the `reviewed by \@name` syntax to the NEWS items and have
added @ErinBecker and @klbarnes20 to their respective reviews
@zkamvar
Copy link
Contributor Author

zkamvar commented Aug 28, 2023

Since Kelly gave it a positive review and the changes were nonblocking I will merge this.

Thank you for your review @klbarnes20!

@zkamvar zkamvar merged commit 0d36595 into main Aug 28, 2023
12 checks passed
@zkamvar zkamvar deleted the allow-overview-pages branch August 28, 2023 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workshop Overview Pages (without episodes) should be allowed
2 participants