-
Notifications
You must be signed in to change notification settings - Fork 360
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
WX-1873 Replace output move option with "copy & update output location" #7565
Conversation
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.
Awesome to see how neatly the move changes evolved into this.
case object FinalWorkflowOutputsMode extends WorkflowOption("final_workflow_outputs_mode") { | ||
// Default to Copy because that was originally the only behavior | ||
def fromString(s: Option[String]): FinalWorkflowOutputsMode = | ||
case object FinalWorkflowOutputsDirMetadata extends WorkflowOption("final_workflow_outputs_dir_metadata") { |
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.
As long as it's well-documented (which it looks like it is) I think this is OK as is, but agree that the naming is awkward. Might be easier to name if it were a boolean, something like FinalWorkflowOutputsDirStoredInOutputs
? 😬
Description
Replaces #7472
As I complained in Slack, I dislike a lot of the naming in this PR. Suggestions welcome.
Release Notes Confirmation
CHANGELOG.md
CHANGELOG.md
in this PRCHANGELOG.md
because it doesn't impact community usersTerra Release Notes