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

Fixing json configuration export #2305

Merged
merged 3 commits into from
Mar 3, 2021

Conversation

eddynaka
Copy link
Collaborator

@eddynaka eddynaka commented Mar 2, 2021

No description provided.

@@ -0,0 +1,5 @@
{
"SARIF2002.ProvideMessageArguments.Options": {
"RuleEnabled": "error"
Copy link
Member

Choose a reason for hiding this comment

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

error [](start = 20, length = 5)

Shouldn't this be cased as 'Error'? Also, why are we fixing the JSON configuration, how did this come up? Note that we can't actually deserialize our own JSON format!! I can explain why offline, the reason is that we can't apply the type information in the JSON (the original XML design uses element attributes for this. To make everything work, we'd need a new JSON serializer/deserializer that persist the type data as special properties at each level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With this change both Error or error would work.
With this change we would be able to export the configuration as json and import.

The issue was that we were serializing the enum as value instead of string, which causes the issue.


In reply to: 586522294 [](ancestors = 586522294)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have a few tests locally that i was using a custom xml. Then i was trying a custom config in json and i saw this error


In reply to: 586535491 [](ancestors = 586535491,586522294)

@eddynaka eddynaka merged commit 02f96e0 into main Mar 3, 2021
@eddynaka eddynaka deleted the users/ednakamu/fixing-json-configuration-export branch March 3, 2021 16:01
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