Remove `failure_reason` and use `identifier` instead
Background
Based on discussion in !132349 (comment 1595471728), failure_reason
doesn't seem to be needed anymore given that we have an identifier
for each mergeability check. We can use identifier
instead when returning which check failed which is currently defined as failed_reason
.
As of this writing, we also have !132721 (closed) which implements failure_reason_text
which is exposed as failureReason
in GraphQL. If we remove the existin failure_reason
methods, we could also rename failure_reason_text
to failure_reason
.
Proposal
- Remove
MergeRequests::Mergeability::CheckBaseService.failure_reason
andMergeRequests::Mergeability::CheckBaseService#failure_reason
. - When a check fails (by calling
MergeRequests::Mergeability::CheckBaseService#failure
), remove the need to pass thefailure_reason
asreason
since we are already passing theidentifier
. - Rename
MergeRequests::Mergeability::RunChecksService#failure_reason
toMergeRequests::Mergeability::RunChecksService#failed_check
and fetch theidentifier
instead ofreason
. Return#failed_check
asfailed_check
when callingServiceResponse.error
in#execute
. - Modify
MergeRequests::Mergeability::DetailedMergeStatusService#execute
to getfailed_check
frompayload
whencheck_results
isn't asuccess?
. - Modify
MergeRequests::Mergeability::DetailedMergeStatusService#ci_check_failure_reason
to fetchidentifier
frompayload
instead ofreason
.
Edited by Patrick Bajao