Skip to content
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

Create feature flag for Grouped security updates #8529

Merged

Conversation

ryanbrandenburg
Copy link
Contributor

This creates a feature flag for Grouped security updates.

@ryanbrandenburg ryanbrandenburg force-pushed the dev/rybrande/GroupUpdate branch from 9365316 to e9ccc8c Compare December 5, 2023 00:26
@@ -15,6 +15,7 @@ class CreateGroupSecurityUpdatePullRequest
include GroupUpdateCreation

def self.applies_to?(job:)
return false if Dependabot::Experiments.enabled?(:grouped_security_updates_disabled)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recognize that it's a bit weird to phrase it like this, but I need the default case to be grouped_security_updates being on and the way Dependabot::Experiments works is it's falsy by default.

@@ -209,8 +209,12 @@
context "when there is an exception that blocks PR creation (cloud)" do
before do
allow(api_client).to receive(:create_pull_request).and_raise(StandardError, "oh no!")
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_ecosystem_versions).and_return(true)
Copy link
Contributor Author

@ryanbrandenburg ryanbrandenburg Dec 5, 2023

Choose a reason for hiding this comment

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

As far as I can tell doing it this way really only worked while we had a single call against enabled?. Since we already had the reset! method I figured we could make use of it instead of mocking things so that later levels of abstraction have an easier time handling things.

@@ -232,10 +236,6 @@
end
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These explicit mocks were not needed once we stop mocking (default to false), so I deleted them.

@ryanbrandenburg ryanbrandenburg marked this pull request as ready for review December 5, 2023 17:39
@ryanbrandenburg ryanbrandenburg requested a review from a team as a code owner December 5, 2023 17:39
Copy link
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

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

Thanks!

@jakecoffman jakecoffman enabled auto-merge (squash) December 5, 2023 18:13
@jakecoffman jakecoffman merged commit 1ecdf6d into dependabot:main Dec 5, 2023
81 checks passed
@ryanbrandenburg ryanbrandenburg deleted the dev/rybrande/GroupUpdate branch December 5, 2023 18:54
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.

2 participants