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

Document how converters should provide notifications #15

Closed
ghost opened this issue Sep 26, 2017 · 6 comments
Closed

Document how converters should provide notifications #15

ghost opened this issue Sep 26, 2017 · 6 comments

Comments

@ghost
Copy link

ghost commented Sep 26, 2017

Copied from sarif-standard/sarif-spec-v1#144, created by @michaelcfanning:

Should we provide a way for a converter to communicate that something strange happened during conversion? What about tools that process existing log files and add some notes/observations? Not a breaking change to extend the enum.

@ghost
Copy link
Author

ghost commented Sep 26, 2017

Comment from @lgolding:

These belong in toolNotifications. Converters can specify a distinguished notification.id. It could even be something like PREfastConverter.1, .2, ...

The message should also make it clear who produced the notification.

@ghost
Copy link
Author

ghost commented Sep 26, 2017

Comment from @lgolding:

@michaelcfanning Do you agree that these should be toolNotifications with a distinguished id, or do you have another suggestion?

@ghost
Copy link
Author

ghost commented Sep 27, 2017

Notes from TC con call 2017/09/27:

  • Need to mark a file as having been the result of a conversion.
  • Need to distinguish notifications produced by the source tool from those produced by the converter.

JimKupsch suggests "provenance" -- tell which native tool output file was converted, and which converter version was used to convert it.

Do we want a chain of these?

toolNotification could say which numeric index into the provenance array was the source of the notification.

@DerSaidin
Copy link

DerSaidin commented Sep 27, 2017

A tool could merge multiple input reports. A linear chain may be insufficient.

@michaelcfanning michaelcfanning added the CSD.1 Will be fixed in CSD.1. label Feb 1, 2018
@michaelcfanning
Copy link
Contributor

another possibility is adding converter specific notifications property

@ghost
Copy link
Author

ghost commented Mar 6, 2018

@michaelcfanning I like the idea of a "converter specific notifications property". Now that we have a conversion object, that's the natural place for it conversion.notifications. I'll write the change draft that way.

ghost pushed a commit that referenced this issue Mar 6, 2018
@ghost ghost self-assigned this Mar 6, 2018
ghost pushed a commit that referenced this issue Mar 14, 2018
@ghost ghost added resolved-fixed and removed question labels Mar 14, 2018
@ghost ghost closed this as completed Mar 14, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants