-
Notifications
You must be signed in to change notification settings - Fork 143
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
Refactored the Startup application type to apply to all OS's #1011
Conversation
… event class has meaning across all of them. Removed the macOS extension as this refactoring removes the only item that was used in the extension.
}, | ||
"is_array": true, | ||
"type": "integer_t" | ||
}, |
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 prior mac_os
extension version had this attribute with a plural for array but it wasn't an array. Is this supposed to be an array? Do we expect for a single startup type, to have multiple app components? It seems like this list needs to be reviewed. E.g. would we have an array with File System Driver
Kernel Driver
and Recognized Driver
at once? We wouldn't have Own Process
and Shared Process
.
And Autoload
still says macOS
in the description.
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.
Correct, in my original pr, the attribute was not an array but this latest fix changes that. The list was taken from the original definition in the ICD schema (where it was also an array). As for the macOS in the description for Autoload, I will make that more generic.
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 think this part of the class needs to be remodeled very soon. We should consider having a driver object if we want an array of drivers that are loaded at startup. The current array of types doesn't tell me what the driver was (just adapter
or something that isn't an app type like shared process
which is really a mode of execution, not an app type). Can we see an example of what is returned from the query?
Otherwise, to get the rest of the query classes in for 1.2, we would need to remove this startup_app class and work on it for 1.3.
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.
@maxhotta what is the status on this one?
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.
@zschmerber Closing this ticket now. I'll be preparing a new pr for this event class.
Refactored the Startup application type to apply to all OS's, as this event class has meaning across all of them.
Removed the macOS extension as this refactoring removes the only item that was used in the extension.
Creating a new PR due to merge conflicts with the original.