[feat] Add new template "type" for SEDAR reports#511
Conversation
New version checklist
|
Checklist
|
Code Metrics Report
Code coverage of files in pull request scope (41.4%)
Reported by octocov |
c5ca77b to
c092a55
Compare
…skeleton except author = SEDAR only (no scientists' names added)
| if (rerender_skeleton){ | ||
| current_folder <- subdir | ||
| } else { | ||
| current_folder <- file.path(asar_folder, |
There was a problem hiding this comment.
It's the same? I'm wondering if this is the case, then we remove functionality for type = "SEDAR" and socialize that the standard guidelines match SEDAR reports already and set the authorship part in the yaml to activate when office = "SEFSC" instead.
What assessments did you look at to determine this is the same? With the hesitation we got, I assumed there would be some differences.
There was a problem hiding this comment.
This suggestion came from the SEFSC workshop, in the context of the authorship difference. I don't think the intent was to provide a report that matched the SEDAR structure since it seems highly different from the SAR.
I can change this PR to remove "type" and instead update authorship when office = SEFSC 👍
There was a problem hiding this comment.
Ah okay that makes this much more clear. This LGTM then. I did not run a test on it so let me do that then I will approve
Schiano-NOAA
left a comment
There was a problem hiding this comment.
This is looking good, but it makes me wonder if this is needed. If there is no actual template, it might be worth more socializing that they are the same. The PR would then only make changes when office = "SEFSC" for authorship in the yml
…= SEFSC; update documentation
|
@sbreitbart-NOAA please also make changes to the citation. Idk how involved that will be, but it still inputs author and I think this doesn't align with SEDAR? Not sure though |
What is the feature?
How have you implemented the solution?
create_template()andcreate_yaml()Does the PR impact any other area of the project, maybe another repo?