[feat]: update_report() - #563
Schiano-NOAA wants to merge 19 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
1579767 to
9542924
Compare
…ather than interactive question
|
Note: tests in this use the |
| #' and figures Quarto documents. | ||
| #' | ||
| #' Default: FALSE | ||
| #' @returns Creates a new folder of pre-filled assessment report files for the |
There was a problem hiding this comment.
I think this file needs a details section (or something similar) that more thoroughly explains what the function is capable of. For example, offering which parts of a report are eligible and likely to be updated: year, model results, etc.
There was a problem hiding this comment.
I'll make this update as you review
|
I'll hold off further reviewing because the function errors without adding in a |
|
@sbreitbart-NOAA After this merges in and the bug for the bib_file is merged, I will do a release. Most likely monday or whenever this is merged in next week |
* Add functionality to rename legacy figure/table docs with rerender_skeleton(); fix bug where tables docs weren't renamed in create_template() * Remove code from create_template() associated with legacy fig/table names
| # section chunks | ||
| # TODO: reset author section in skeleton -- remove all previous authorship (does this work?) | ||
| # Update skeleton with new year, authors, model results, region, if added | ||
| rerender_skeleton( |
There was a problem hiding this comment.
I'm not able to add new sections. I think this is failing in rerender_skeleton(), not update_report() itself. Ex:
dir.create("test_report")
create_template(bib_file = F)
update_report(previous_file_dir = "report", file_dir = "test_report", new_section = "Special Section", section_location = "after-introduction")
| cli::cli_alert_info("Figures document reset to default.") | ||
| } | ||
|
|
||
| if (reset_tables_and_figures) { |
There was a problem hiding this comment.
is there a reason for having two if statements when it appears that these could be combined?
| ) | ||
| ) | ||
| # Create figures doc | ||
| create_figures_doc( |
There was a problem hiding this comment.
the figures and tables docs aren't being named correctly if type isn't "sar" (i.e., they're being named as if type = "sar").
In create_template(), the files are renamed based on type. But if type isn't provided here, one solution could be to identify type based on the child docs present (e.g., 08_abc signals type = "safe"), then rename accordingly
| # TODO: type - this needs to just pull all files from folder that | ||
| # it was copying from when custom sections is null -- DONE |
There was a problem hiding this comment.
| # TODO: type - this needs to just pull all files from folder that | |
| # it was copying from when custom sections is null -- DONE |
sbreitbart-NOAA
left a comment
sbreitbart-NOAA
left a comment
There was a problem hiding this comment.
Most of this works very well- just a few things to remediate before merging 👍
Code Metrics Report
Code coverage of files in pull request scope (65.1%, patch 62.4%)
Reported by octocov |
This branch has not been deployed
2 participants
What is the feature?
How have you implemented the solution?
Does the PR impact any other area of the project, maybe another repo?