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

Disentangle bookdown/coursera/leanpub rendering. #459

Merged
merged 8 commits into from
Feb 11, 2022

Conversation

cansavvy
Copy link
Collaborator

@cansavvy cansavvy commented Feb 11, 2022

Purpose/implementation Section

What changes are being implemented in this Pull Request?

render_coursera() function is actually just "render_without_toc" but Leanpub also needs a toc-less version.

I'm working on disentangling the renders from each other so someone can truly specify just one and not the others. Before Leanpub was dependent on Coursera which was dependent on Bookdown.

Now, my hope is you can really just choose one and not the others (although really Bookdown is still run underneath the hood for all)

Over in the ottr package, I renamed render_coursera to render_without_toc() so this is less confusing. jhudsl/ottrpal#96

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2022

No quiz errors detected! 🎉
Comment updated at 2022-02-11 with changes from eb11c94

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2022

No spelling errors! 🎉
Comment updated at 2022-02-11 with changes from eb11c94

@cansavvy cansavvy changed the title Rearrange render-all for clarity Disentangle bookdown/coursera/leanpub rendering. Feb 11, 2022
render-coursera:
name: Render Coursera
needs: [yaml-check, render-bookdown]
render-tocless:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a new section that will run before Leanpub or Coursera if either is set to yes

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it makes sense to call this something like "Start Leanpub/Coursera render" and then have the jobs below be "Finish Leanpub" and "Finish Coursera"

run: |
Rscript -e "ottr::bookdown_to_embed_leanpub( \
Rscript -e "ottr::bookdown_to_embed_leanpub(
render = FALSE \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I realized its redundant to have bookdown re-rendered if we just did it. So we're using the option here to skip the bookdown::render() step done by this function if bookdown was just run previously.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. I just realized it never needs to be re-rendered here because the render-tocless section does this.

@cansavvy cansavvy requested a review from avahoffman February 11, 2022 14:40
Copy link
Contributor

@avahoffman avahoffman left a comment

Choose a reason for hiding this comment

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

Looks good!

.github/workflows/pull_request.yml Show resolved Hide resolved
render-coursera:
name: Render Coursera
needs: [yaml-check, render-bookdown]
render-tocless:
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it makes sense to call this something like "Start Leanpub/Coursera render" and then have the jobs below be "Finish Leanpub" and "Finish Coursera"

@cansavvy
Copy link
Collaborator Author

I wonder if it makes sense to call this something like "Start Leanpub/Coursera render" and then have the jobs below be "Finish Leanpub" and "Finish Coursera"

Yes, I like this.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2022

Re-rendered previews from the latest commit:

Updated at 2022-02-11 with changes from eb11c94

@cansavvy cansavvy merged commit 0637d3a into main Feb 11, 2022
@cansavvy cansavvy deleted the cansavvy/rearrange-renders branch February 11, 2022 16:34
avahoffman added a commit to jhudsl/AnVIL_Book_Instructor_Guide that referenced this pull request Feb 14, 2022
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.

2 participants