-
Notifications
You must be signed in to change notification settings - Fork 954
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
Clean Up Composite UI #610
Conversation
…ner beforehand for each step)
@@ -23,109 +23,113 @@ public sealed class ScriptHandler : Handler, IScriptHandler | |||
|
|||
public override void PrintActionDetails(ActionRunStage stage) | |||
{ | |||
if (stage == ActionRunStage.Post) | |||
// We don't want to display the internal workings if composite (similar/equivalent information can be found in debug) | |||
if (!ExecutionContext.InsideComposite) |
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.
All I did was wrap all the existing code in this if statement
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.
add the if
check to the caller of PrintActionDetails
to reduce diff?
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.
or should we print all these info using ExecutionContext.Debug()
instead of ExecutionContext.Output()
?
@@ -23,109 +23,113 @@ public sealed class ScriptHandler : Handler, IScriptHandler | |||
|
|||
public override void PrintActionDetails(ActionRunStage stage) | |||
{ | |||
if (stage == ActionRunStage.Post) | |||
// We don't want to display the internal workings if composite (similar/equivalent information can be found in debug) |
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.
var writeDetails = ExecutionContext.InsideComposite ? ExecutionContext.Debug : ExecutionContext.Output as Action<string>;
...
writeDetails($"##[group]Run {firstLine}");
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 tried using delegate stuff but couldn't figure it out / maybe it's impossible (???).
I'm just going to do this instead since it accomplishes the same thing and may be easier to read:
void writeDetails(string message)
{
if (ExecutionContext.InsideComposite)
{
ExecutionContext.Debug(message);
}
else
{
ExecutionContext.Output(message);
}
}
* Remove redundant code (display name is already evaluated in ActionRunner beforehand for each step) * remove * Remove nesting information for composite steps. * put messages in debug logs if composite. if not, put these messages as outputs * Fix group issue * Fix end group issue
* Remove redundant code (display name is already evaluated in ActionRunner beforehand for each step) * remove * Remove nesting information for composite steps. * put messages in debug logs if composite. if not, put these messages as outputs * Fix group issue * Fix end group issue
* Remove redundant code (display name is already evaluated in ActionRunner beforehand for each step) * remove * Remove nesting information for composite steps. * put messages in debug logs if composite. if not, put these messages as outputs * Fix group issue * Fix end group issue
* Remove redundant code (display name is already evaluated in ActionRunner beforehand for each step) * remove * Remove nesting information for composite steps. * put messages in debug logs if composite. if not, put these messages as outputs * Fix group issue * Fix end group issue
In this PR, I remove all of the "extra" information for each run step inside of the composite action.
It's easier to see through an example:
As you can see, our existing code has the equivalent information written to the debugging console as the "extra" information node.
Before Changes:
After Changes: