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

Consider: 'review' or 'audit' result level. and reconsider 'note' #215

Closed
michaelcfanning opened this issue Aug 4, 2018 · 5 comments
Closed
Labels
design-approved The TC approved the design and I can write the change draft p1 Priority 1 issue to close

Comments

@michaelcfanning
Copy link
Contributor

The SARIF 'result' level doesn't provide for reporting on potentially extremely verbose/noisy issues that may still be relevant in circumstances when developers are motivated to provide a substantive audit or review.

It is helpful to separate these from 'note' which is currently an extremely ambiguous result type, that might flag a candidate pattern for change (or might represent any information at all).

So, a second change is to tighten up the meaning of 'note' or perhaps switch to 'suggestion' or 'information'. We could clarify that in all cases, resultLevel is intended to categorize a potential issue/candidate for code change. Simple observations about code, other logging, should be expressed as tool notifications, instead.

@ghost
Copy link

ghost commented Nov 15, 2018

Per @michaelcfanning's comment in #280: We can also consider the question of a numeric severity property in this issue (#215).

@michaelcfanning
Copy link
Contributor Author

See as well #59. A result could also flag that it is a valid result that occurs in dead code. I believe this is the remaining PolySpace designation not already covered by the format (since we added 'open' as an explicit result level.

@michaelcfanning michaelcfanning added p1 Priority 1 issue to close design-approved The TC approved the design and I can write the change draft labels Jan 24, 2019
@ghost
Copy link

ghost commented Feb 20, 2019

@michaelcfanning We added result.kind value "review" as part of #317, "Consider splitting resultLevel into result.level and result.kind". I'd forgotten it was a separate issue.

We still have the "note" question to consider.

@michaelcfanning
Copy link
Contributor Author

Nope, we considered this issue in the f2f and determined that that addition of 'review' (and now the addition of 'debug') obviates the need to reconsider 'note'. 'note' stands as defined, it is the least severe failure that may be returned by analysis (but it does, in fact, constitute a failure.

To get really picky, the concept we have stopped discussing is 'observation', that is, a format report of some kind that is perfectly neutral/factual, i.e., neither pass nor fail.

I am closing this. Please review. If you feel like observation is important, let's file a new issue.

@ghost
Copy link

ghost commented Feb 21, 2019

I do not feel strongly enough about this to argue for adding "observation". It's non-breaking if we add it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-approved The TC approved the design and I can write the change draft p1 Priority 1 issue to close
Projects
None yet
Development

No branches or pull requests

1 participant