Introduce feature flag to raise exception on same branch exists #10878
Conversation
sachin-sandhu
force-pushed
the
ssandhu/raise-exception-on-same-name-branch
branch
from
0c787a9 to
acef865
Compare
| @@ -580,6 +594,11 @@ def raise_custom_error(base_err, type, message) | |||
| raise type, message | |||
| end | |||
| end | |||
|
|
|||
| sig { returns(T::Boolean) } | |||
| def trace_log? | |||
There was a problem hiding this comment.
can we rename this method to something more descriptive?
There was a problem hiding this comment.
@Nishnha , changed to more descriptive
| ) | ||
| raise DuplicateBranchExists, "Duplicate branch #{branch_name} already exists" | ||
| end | ||
|
|
There was a problem hiding this comment.
If we are raising when a branch already exists then we can get rid of the checks for when an unmerged pull request exists. But it might be better to do that in a follow-up PR since this one is already a lot of changes
There was a problem hiding this comment.
@Nishnha , ill do it next release
| @@ -443,5 +458,11 @@ def includes_security_fixes? | |||
| def requirements_changed?(dependency) | |||
| (dependency.requirements - T.must(dependency.previous_requirements)).any? | |||
| end | |||
|
|
|||
| sig { returns(T::Boolean) } | |||
| def trace_log? | |||
There was a problem hiding this comment.
I don't think we really need to do any logging in this file.
There was a problem hiding this comment.
@Nishnha , removed logging from the file.
| @@ -132,6 +139,13 @@ def require_up_to_date_base? | |||
| # rubocop:disable Metrics/PerceivedComplexity | |||
| sig { params(name: String).returns(T::Boolean) } | |||
| def branch_exists?(name) | |||
| if trace_log? | |||
There was a problem hiding this comment.
this log doesn't seem particularly helpful since we can see if it's a duplicate from the job log in #create, and the sentry error that it raises
There was a problem hiding this comment.
If we want to know what the branch name is we should maybe change this to a debug statement so it doesn't get printed out with every job log when we remove the ff
There was a problem hiding this comment.
@Nishnha , i have changed to debug now.
sachin-sandhu
force-pushed
the
ssandhu/raise-exception-on-same-name-branch
branch
from
7d328d2 to
8d0b0cb
Compare
sachin-sandhu
force-pushed
the
ssandhu/raise-exception-on-same-name-branch
branch
from
8d0b0cb to
a9ba774
Compare
What are you trying to accomplish?
This is related to issue where dependabot can push changes to existing branches. This can result in unwanted changes to be included with existing branch.
Fix: We are introducing change under a feature flag to return an exception in case
finds an existing branch with same name. The impact will be monitored consistency before making this change permanent.
Anything you want to highlight for special attention from reviewers?
How will you know you've accomplished your goal?
Checklist