Skip to content

Update preamble prompt#430

Merged
sbreitbart-NOAA merged 2 commits intodevfrom
rephrase-preamble
Mar 4, 2026
Merged

Update preamble prompt#430
sbreitbart-NOAA merged 2 commits intodevfrom
rephrase-preamble

Conversation

@sbreitbart-NOAA
Copy link
Collaborator

What is the feature?

How have you implemented the solution?

  • Clarifying the prompt

Does the PR impact any other area of the project, maybe another repo?

  • No

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

Checklist

  • PR base branch is accurate
  • Is the code concise?
  • Comments are clear and useful.
  • Can you remove or combine any arguments?
  • Do argument contain defaults (if appliable)?
  • Code is documented and example provided (Roxygen).
  • Did you make a test (testthat)?
  • Was this tested under multiple scenarios?
  • Did you run devtools::check()?

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

Code Metrics Report

Coverage Code to Test Ratio Test Execution Time
37.1% 1:0.2 28s

Code coverage of files in pull request scope (40.4%)

Files Coverage
R/create_template.R 40.4%

Reported by octocov

Copy link
Collaborator

@Schiano-NOAA Schiano-NOAA left a comment

Choose a reason for hiding this comment

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

Can you change the message so it matches the one in the issue? I think the current language is not accurate. We don't want to say they "may" need a new preamble, but more so it will be overwritten so do you want to keep it or not. I think the language in the issue "Update the preamble to match entered arguments? (Y/N)" is closer to what we want here

@sbreitbart-NOAA
Copy link
Collaborator Author

Can you change the message so it matches the one in the issue? I think the current language is not accurate. We don't want to say they "may" need a new preamble, but more so it will be overwritten so do you want to keep it or not. I think the language in the issue "Update the preamble to match entered arguments? (Y/N)" is closer to what we want here

The reason why I didn't take the language from the issue is that it will change the direction of the question. Currently, "yes" would entail keeping the preamble, and with the "update the preamble..." question, "yes" would entail changing the preamble. I thought the user may find this confusing if they've already grown accustomed to it, but it's not anything as major as a breaking change- just something to be aware of.

If you're alright with that, I can change the question to the one from the issue 👍

@Schiano-NOAA
Copy link
Collaborator

Schiano-NOAA commented Mar 3, 2026 via email

@sbreitbart-NOAA sbreitbart-NOAA merged commit c265f76 into dev Mar 4, 2026
1 check passed
@sbreitbart-NOAA sbreitbart-NOAA deleted the rephrase-preamble branch March 4, 2026 14:21
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.

[Bug]: 'output_and_quantities' chunk in skeleton missing line to load in model results

2 participants