Skip to content

Conversation

@RachelElysia
Copy link
Member

@RachelElysia RachelElysia commented May 22, 2025

Issue

Part of fleetdm/fleet#29293

Description

  • This PR takes all current files in this repo and puts them into subdirectories consistent with: https://github.com/fleetdm/fleet/tree/main/it-and-security
  • Removes lib/uninstall-zoom.script.sh as it was 1) empty and 2) is confusing if it's part of a general script or a software's uninstall script
  • Does not move /lib/agent-options.yml into /lib/agent-options/agent-options.yml as it's fails gitops CI test moving it

Copy link
Member

@getvictor getvictor left a comment

Choose a reason for hiding this comment

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

Changes make sense to me.

FYI: @noahtalerman @eugkuo regarding this customer-facing change.

@RachelElysia RachelElysia marked this pull request as ready for review May 27, 2025 17:19
@RachelElysia
Copy link
Member Author

Thanks @getvictor ! I added 2 installer yml examples (one for windows one for macos) since your approval if you have a moment

Copy link
Member

@getvictor getvictor left a comment

Choose a reason for hiding this comment

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

Why not simply point people to these examples in the fleet repo? If we're putting configs into this repo, the customer expectation would be that they actually work, which means we are continually testing them for regressions. That's not the case -- we only have smoke tests of the main configs (free/premium) for this repo.

@noahtalerman
Copy link
Member

Why not simply point people to these examples in the fleet repo? If we're putting configs into this repo, the customer expectation would be that they actually work, which means we are continually testing them for regressions. That's not the case -- we only have smoke tests of the main configs (free/premium) for this repo.

Agreed. @RachelElysia I think we want to keep this GitOps repo as light as possible so that when a customer tries it, it always works.

Maybe we remove the software examples to get this PR merged in? We can remove other stale examples in a follow up pass.

@noahtalerman
Copy link
Member

@RachelElysia also heads up that @harrisonravazzolo has been using this repo during GitOps training sessions. I added Harry as a reviewer to this PR. I think we want his eyes before we merge in the changes.

For this reason, I also updated CODEOWNERS such that Harry owns the best practice file structure: #70

@RachelElysia
Copy link
Member Author

Thanks for the review @getvictor and context @noahtalerman !

@harrisonravazzolo As DRI, please confirm and update this task so I'm not aiming in the dark of what is being asked fleetdm/fleet#29293 -- Grazie!

cc: @mostlikelee

@@ -1 +0,0 @@
# This will be a script that uninstalls Zoom from macOS hosts. No newline at end of file
Copy link
Member Author

Choose a reason for hiding this comment

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

@harrisonravazzolo

We currently have this very generic example of uninstall zoom script in here. Is that preferred than an actual script?
That's an option for keeping the santa.yml or slack.yml as a reference. lmk!

Copy link
Member

@harrisonravazzolo harrisonravazzolo left a comment

Choose a reason for hiding this comment

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

All make sense to me @RachelElysia - thanks for checking and apologies for the delay, was traveling yesterday!

@RachelElysia
Copy link
Member Author

@harrisonravazzolo when you merge this in, can you leave a comment on fleetdm/fleet#29293 saying that you QAed for transparency <3

@harrisonravazzolo
Copy link
Member

@RachelElysia - sure thing. going to merge this afternoon

@harrisonravazzolo harrisonravazzolo merged commit 7dbc77c into main May 30, 2025
1 check passed
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.

5 participants