-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fixed version of #1346: Allow split_by to accept numeric values #1490
base: main
Are you sure you want to change the base?
Conversation
Squashed commit of the following: commit 99f8698 Author: Laure Cougnaud <[email protected]> Date: Tue Jun 14 10:33:16 2022 +0000 limit numeric split_by to: [1:6] + update documentation commit 3304d5a Merge: 7f1e38b 4cdf9c6 Author: Laure Cougnaud <[email protected]> Date: Tue Jun 14 08:20:17 2022 +0000 merge branches Merge remote-tracking branch 'OAGithubRStudio/main' into split_by-numeric # Conflicts: # R/gitbook.R # R/html.R # man/build_chapter.Rd # man/epub_book.Rd # man/gitbook.Rd # man/html_chapters.Rd # man/html_document2.Rd # man/pdf_book.Rd # man/publish_book.Rd # man/render_book.Rd # man/resolve_refs_html.Rd # man/serve_book.Rd commit 7f1e38b Author: lcougnaud <[email protected]> Date: Mon Nov 4 11:03:18 2019 +0100 fix for nested subsection (part 1) commit ee342c7 Author: lcougnaud <[email protected]> Date: Thu Oct 31 16:17:33 2019 +0100 fix specification split_by commit 60bb103 Author: lcougnaud <[email protected]> Date: Thu Oct 31 14:25:09 2019 +0100 fix inclusion subsections commit 9c11691 Author: lcougnaud <[email protected]> Date: Thu Oct 24 18:05:31 2019 +0200 split_chapters: fix numbering higher level sections that split_by commit 0442817 Author: lcougnaud <[email protected]> Date: Thu Oct 24 17:42:57 2019 +0200 update split_chapters for numeric 'split_by'
Same content as #1346 but with the failing tests fixed :-) |
You can cleanly cherry pick every thing after the |
@yihui The code in this one is ready for review btw. Not sure if that was clear from my above comments. The commit messages certainly leave something to be desired. I can fix them and make sure @lcougnaud is show as the author of the code they wrote if that matters. Let me know. I figured those messages would get squashed away anyhow. |
@katrinabrock thanks for fixing the tests & finalizing the changes! For me it is all fine to squash everything into one single commit during the merging. |
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.
To be honest, I feel this is beyond my capability to review, and I'll simply trust your hard work. Before I merge it, could you test a couple of books and see if they render to the same HTML files before/after this PR? For example:
- https://github.com/rstudio/bookdown/tree/main/inst/examples
- https://github.com/rstudio/blogdown/tree/main/docs
- https://github.com/rstudio/rmarkdown-book
You can first render a book without this PR, temporarily commit the HTML output files in Git, and then install bookdown with this PR (and restart R), render again. Then see if there are any changes in Git.
Thanks!
ok! yes, I can do that. I will look also take closer at the code written by @lcougnaud before you blindly merge so at least there are two sets of eyes on it. |
Great! If you run into any errors when building these books, you can add |
Close #1346