-
Notifications
You must be signed in to change notification settings - Fork 558
Update policy queries for RPM/Debian based systems #25026
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
Conversation
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.
Can't approve this anyway so my feedback is non-blocking, but thanks for updating the queries here. Checked API docs and those are good to go. I'll defer to Marko/Noah on which text changes make sense.
@@ -73,7 +73,7 @@ SELECT 1 FROM apps WHERE name = '<SOFTWARE_TITLE_NAME>' AND version_compare(bund | |||
SELECT 1 FROM programs WHERE name = '<SOFTWARE_TITLE_NAME>' AND version_compare(version, '<VERSION>') >= 0; | |||
``` | |||
|
|||
### Ubuntu (deb) | |||
### Debian-based (deb) |
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.
Maybe keep the heading as-is and add text indicating that this works on other Debian-based (Debian itself, Mint, etc.) distros, and same thing with RPM? Also probably want to change L62 to be 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 added one more line to enumerate some of the supported OSs (didn't change back to Ubuntu because the query is actually more broad).
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.
And the new automatic install feature is using the "Debian-based" and "RPM-based" terms.
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.
Fair enough. Maybe we drop the (deb) then?
Non-blocking feedback though.
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 it makes sense to keep deb
and rpm
extensions as we use them in UI.
#24385 Some docs change here: #25026. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [X] Added/updated tests - [X] Manual QA for all new/changed functionality
Docs update for #25021/#24385.