Skip to content
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

Enable automation details for command base #2407

Merged
merged 8 commits into from
Nov 5, 2021

Conversation

eddynaka
Copy link
Collaborator

@eddynaka eddynaka commented Nov 4, 2021

Description

Imagine SARIF Pattern Matcher being run by someone, and we want to automatically ingest the output SARIF somehow.

With this, we could ask the user to run SARIF Pattern Matcher and pass --automationId and --automationGuid, that information would be stored in the SARIF and we would know that the SARIF was generated by a specific team.

Follow-up change

Change the commandBase to accept a URI and this URI will be used to send the generated SARIF to that endpoint.

cc: @cfaucon

@@ -220,6 +220,14 @@ public bool ShouldSerializeGraphs()
return this.Graphs.HasAtLeastOneNonDefaultValue(Graph.ValueComparer);
}

public bool ShouldSerializeAutomationDetails()
{
return this.AutomationDetails?.Description != null ||
Copy link
Member

Choose a reason for hiding this comment

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

AutomationDetails?.Description

Why should we serialize an empty description? suggest using is null or whitespace...

Copy link
Collaborator Author

@eddynaka eddynaka Nov 5, 2021

Choose a reason for hiding this comment

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

The Description isn't a string, it is a Message type. It's why I didn't change to IsNullOrWhiteSpace check

/// </summary>
public partial class RunAutomationDetails
{
public bool ShouldSerializeDescription() => this.Description != null;
Copy link
Member

Choose a reason for hiding this comment

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

Description

?? why persist an empty description? how is this useful?

Copy link
Collaborator Author

@eddynaka eddynaka Nov 5, 2021

Choose a reason for hiding this comment

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

The Description isn't a string, it is a Message type. It's why I didn't change to IsNullOrWhiteSpace check

public string BaselineSarifFile { get; set; }

[Option(
"automationId",
Copy link
Member

@michaelcfanning michaelcfanning Nov 4, 2021

Choose a reason for hiding this comment

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

automationId

why aren't these in common options base? won't we need them for other commands? i guess we can move them later if so. #Resolved

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just moved! :)

Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

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

:shipit:

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.

2 participants