-
Notifications
You must be signed in to change notification settings - Fork 94
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
Fix NotYetAutoGenerated folder out of date issue by regenerate and sync the changes #2537
Conversation
This pull request fixes 5 alerts when merging c7f8175 into 025f64c - view on LGTM.com fixed alerts:
|
src/ReleaseHistory.md
Outdated
@@ -6,6 +6,7 @@ | |||
* BUGFIX: Remove duplicated rule definitions when executing `match-results-forward` commands for results with sub-rule ids. [#2486](https://github.com/microsoft/sarif-sdk/pull/2486) | |||
* BUGFIX: Update `merge` command to properly produce runs by tool and version when passed the `--merge-runs` argument. [#2488](https://github.com/microsoft/sarif-sdk/pull/2488) | |||
* BUGFIX: Eliminate `IOException` and `DirectoryNotFoundException` exceptions thrown by `merge` command when splitting by rule (due to invalid file characters in rule ids). | |||
* BUGFIX: Fix NotYetAutoGenerated folder out of date issue by regenerate and manually sync the changes. [#2537](https://github.com/microsoft/sarif-sdk/pull/2537) |
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.
Fix NotYetAutoGenerated folder out of date issue by regenerate and manually sync the changes.
How does this bug present from the customer's perspective? BUGFIX release history details should be of the format:
BUGFIX: Fix <symptom from customer's perspective> [by <change details if appropriate>]...
#Resolved
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.
right, the "NotYetAutoGenerated folder out of date" is not from customer's perspective.
how about:
BUGFIX: Fix classes inside NotYetAutoGenerated folder missing virtual
keyword for public methods and properties, by regenerate and manually sync the changes.
This pull request fixes 5 alerts when merging 802a92b into 025f64c - view on LGTM.com fixed alerts:
|
This pull request fixes 5 alerts when merging 1da6357 into af69a18 - view on LGTM.com fixed alerts:
|
@@ -5,7 +5,8 @@ | |||
* BUGFIX: Resolve issue where `match-results-forward` command fails to generate VersionControlDetails data. [#2487](https://github.com/microsoft/sarif-sdk/pull/2487) | |||
* BUGFIX: Remove duplicated rule definitions when executing `match-results-forward` commands for results with sub-rule ids. [#2486](https://github.com/microsoft/sarif-sdk/pull/2486) | |||
* BUGFIX: Update `merge` command to properly produce runs by tool and version when passed the `--merge-runs` argument. [#2488](https://github.com/microsoft/sarif-sdk/pull/2488) | |||
* BUGFIX: Eliminate `IOException` and `DirectoryNotFoundException` exceptions thrown by `merge` command when splitting by rule (due to invalid file characters in rule ids). | |||
* BUGFIX: Eliminate `IOException` and `DirectoryNotFoundException` exceptions thrown by `merge` command when splitting by rule (due to invalid file characters in rule ids). [#2513](https://github.com/microsoft/sarif-sdk/pull/2513) | |||
* BUGFIX: Fix classes inside NotYetAutoGenerated folder missing `virtual` keyword for public methods and properties, by regenerate and manually sync the changes. [#2537](https://github.com/microsoft/sarif-sdk/pull/2537) |
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.
Try this, which is a little more focused on customer impact:
Restore missing 'virtual' keyword for several SARIF types, previously overwritten by build system
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.
Did we ship with this problem?
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.
The NotYetAutoGenerated folder was out of date,
because I will be using the updated JSchema to regenerate, these out of sync issues will be also mixed in my PR.
It will be easier for reviewer to review by a separating PR:
A PR which did not do any manual change, just make the current code up to date with current JSchema, which should have been done with the previous JSchema change.
Another PR to use the next version of JSchema, for the BigInteger change.
This PR is purely just to make the NotYetAutoGenerated folder up to date with current JSchema, no other changes.
Did not include test because the changes are:
virtual
which was tested in all files those are not in NotYetAutoGenerated folder