MergeRequests::CreateFromIssueService should use standard `params` named argument in the constructor and pass it to the superclass
The following discussion from !59182 (merged) should be addressed:
-
@dzaporozhets started a discussion: What is the reason behind
mr_params
? I assume this will conflict withparams in
MergeRequests::CreateService` methods?Should we leave a comment explaining it?
Yes. I've updated the code with the following comment, which will link to this issue:
# TODO: This constructor does not use the "params:" argument from the superclass,
# but instead has a custom "mr_params:" argument. This is because historically,
# prior to named arguments being introduced to the constructor, it never passed
# along the third positional argument when calling `super`.
# This should be changed, in order to be consistent (all subclasses should pass
# along all of the arguments to the superclass, otherwise it is probably not an
# "is a" relationship). However, we need to be sure that passing the params
# argument to `super` (especially target_project_id) will not cause any unexpected
# behavior in the superclass. Since the addition of the named arguments is
# intended to be a low-risk pure refactor, we will defer this fix
# to this follow-on issue:
# ...