Draft: AI Feature flag removal
What does this MR do and why?
TODO
-
Deal with javascript feature flag usage (camelCased) . Will require updating the regex to find JS files and also we need a custom prompt to explain to the AI how to remove the JS references. -
Fix or removed the aborted
logic- This was added to deal with the problem of running the keep with multiple changes at once. Without the
abort
option housekeeper will always try to create an MR for every change weyield
. We can choose to skip theyield(change)
when we know that the AI did not succeed but this brings a different problem if we've made some edits but not edited everything. Because we also rely onyield(change)
to have housekeeper undo all the changes that we've made. So I was noticing problems here and I introducedabort
but in my testing it didn't seem to be working and I was regularly left with extra files in my diff after running the keep when it aborted changes.
- This was added to deal with the problem of running the keep with multiple changes at once. Without the
-
We should also abort a change if ::Gitlab::Housekeeper::Shell.rubocop_autocorrect(file)
fails. Right now it raises and stops running housekeeper. But it would be ideal if we could run housekeeper with-m 10
and it would just skip any changes that failed for some reason and move on to the next change. This probably requiresabort
to work correctly or some other way to catch these errors and undo all our changes and continue on. -
Add an additional check after all changes are made to grep the codebase one more time. We should abort the change if we find any remaining references to the feature flag -
Extract the abort
andfilter_identifiers
improvements into a separate MR to be merged first as they are general improvements . They probably also need tests -
Run keep for 5 MRs and figure out what problems it's having and add more TODOs for those problems -
Keep iterating until this is more reliable. It doesn't always have to succesfully remove every feature flag but we want to minimize noise and not create too many MRs that are wrong and someone has to take the time to review them. -
Once this keep is reliable we can merge it and start running it in CI -
Update the keep to work out which feature flags are enabled on GitLab.com instead of just relying on the YAML default_enabled: true
. This is necessary because most of the time people aren't even updating the YAML file once they enable it via ChatOps. This will also require us to figure out how to get this data somehow from GitLab. We can do this using the Postgres AI approach we used for::Keeps::OverdueFinalizeBackgroundMigration
as the feature flag state is in the database. But it would be nice if it was easier for people to run this without production DB access. We could maybe also get this data from prometheus which is slightly lower risk and easier to add to our CI environment. Alternatively we could extract the data from comments on the rollout issues as these are also commenting every time the feature flag is changed. -
Add it to CI similar to what is done in gitlab-org/quality/engineering-productivity/team!238 (diffs)
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
Related to #450442
Edited by Siddharth Dungarwal