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
Support output-style property #142
Support output-style property #142
Conversation
Thank you for the contribution! Would you be able to change the string to use Lazy Properties? Should we be defaulting to using single as the default? I would assume that most flank use cases run more than one shard. Is this broken for all cases using more than one shard? |
@runningcode you are welcome!
Of course.
I didn't because it looked weird to me that Fladle overrides some Flank's property's default. As a Fladle user, I'd prefer to default output style to
What do you mean when you say "for all cases"? I tried with just 2 shards and with 50 shards. The issue occurs in both cases. |
Thanks for using the lazy property!
I mean if it improves the experience for 2 shards and 50 shards, I think we should make it the default? The only case where it degrades the experience is for 1 shard? Is that true? |
Yes, as far as I understand. |
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.
Looks great! I will merge this tomorrow.
@Test | ||
fun writeOutputStyle() { | ||
val properties = emptyExtension { | ||
outputStyle.set("anyString") |
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.
i guess this will fail when the flank command is executed?
i'm fine with that, just wanted to point it out. i don't think validating this option on the fladle side makes sense in case more options are introduced by flank down the line.
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.
Yes, I did that on purpose for the same reason you pointed out.
@@ -44,6 +44,7 @@ internal class YamlWriter { | |||
val smartFlankDisableUpload = config.smartFlankDisableUpload | |||
val localResultsDir = config.localResultsDir.orNull | |||
val testTargetsAlwaysRun = config.testTargetsAlwaysRun | |||
val outputStyle = config.outputStyle.orNull |
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.
i think orNull
isn't necessary or has no effect here since the convention is set.
Closes #140