Reportingdescriptor name to string change #1309
Conversation
| "text": "Rules were meant to be broken." | ||
| }, | ||
| "fullDescription": { | ||
| "text": "This is a test for validating modification of tool.driver.ruleDescriptors" |
There was a problem hiding this comment.
tool.driver.ruleDescriptors [](start = 71, length = 27)
i might need a bit of help in coming up with more TCs to ensure all paths are tested. Will sync up offline. I am ok adding those as part of separate PR is required. #WontFix
...Data/SarifVersionOneToCurrentVisitor/ExpectedOutputs/OneRunWithAllReportingDescriptors.sarif
Outdated
Show resolved
Hide resolved
remove all default ranks of -1.0. they should always be elided from persisted content. this content isn't strictly illegal per schema, of course. #Resolved Refers to: src/Sarif.UnitTests/TestData/SarifVersionOneToCurrentVisitor/ExpectedOutputs/OneRunWithRules.sarif:32 in 04763a1. [](commit_id = 04763a1, deletion_comment = False) |
...ests/TestData/SarifVersionOneToCurrentVisitor/Inputs/OneRunWithAllReportingDescriptors.sarif
Outdated
Show resolved
Hide resolved
harleenkohli
changed the title
|
@michaelcfanning we resolved the discussed comments. If there are no additional comments, i love for this one to get merged before i start the final changes for localization removal. Can you please take a look :-) |
This is a WIP. I have to add more TCs to ensure various paths to reach ReportingDescriptor.Name are tested, raising PR to get early feedback on approach.
@michaelcfanning Please review ApplyChangesFromTC32 specifically as its looking a bit complex :)