-
Notifications
You must be signed in to change notification settings - Fork 86
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 RemoveOptionalData for RewriteCommand #2160
Conversation
@@ -72,4 +75,5 @@ Run ```Sarif.Multitool convert --help``` for the current list. | |||
| Name | Purpose | | |||
| ---- | ------- | | |||
| pretty-print | Produce pretty-printed JSON output rather than compact form. | |
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.
form [](start = 72, length = 4)
We should mention this is the default! :) Probably, this argument should be deprecated over time (i.e., we'd only have minify which would reverse our default pretty print behavior).
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 took a look at our base class and pretty-print isn't default. What we could do is: in a separate PR remove the minify since minify is the opposite of pretty-print. What do you think?
In reply to: 529762644 [](ancestors = 529762644)
* FEATURE: Multitool SARIF rewrite accepts `remove` parameter[#2160](https://github.com/microsoft/sarif-sdk/pull/2160) | ||
|
||
## **v2.3.8** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.8) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.8) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.8) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.8) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.3.8) | ||
|
||
* FEATURE: PACKAGE BREAKING: Upgrade from .NET Framework 4.5 to .NET Framework 4.5.2 [#2135](https://github.com/microsoft/sarif-sdk/pull/2135) |
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.
FEATURE [](start = 2, length = 7)
Eliminate whitespace on line 6? Go ahead and add the v2.3.9 banner at the top of the file? #Closed
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 was thinking about this and it may cause some confusion because it will show that we have already the version 2.3.9, which is not true. What i will do is change and create a ## Unreleased.
What do you think?
In reply to: 529763647 [](ancestors = 529763647)
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.
Sure, that's a fine idea. We just need to be disciplined about updating it when we actually release.
In reply to: 529771234 [](ancestors = 529771234,529763647)
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.
Nice work! Left a couple of small suggestions to act on, just update as you think best and merge. #Closed |
Ok, since the pretty-print isn't default , i won't change for now. I'm waiting the pipeline to merge. In reply to: 733135964 [](ancestors = 733135964) |
Changes:
RemoveOptionalData
forRewriteCommand
CodeFlows
asOptionallyEmittedData
With this we would be able to use the following command:
Adding some data: