Add enhanced support for review request matching.
Review Request #12263 — Created April 26, 2022 and submitted — Latest diff uploaded
Our review request matching ("guessing") code has been around for a very
long time. It worked by comparing the summary and description of pending
review requests to the local tree, computing a score, and returning
either an exact match (100% equal) or a fuzzy match. Fuzzy matches would
be presented to the user so that they could determine which is the
correct review request to update.
This change greatly enhances this logic, deprecating a lot of old code
in the process. Review requests can now be matched based on the commit
ID, or based on review request
extra_datacontent (if the
implementation supports it).
Matching is done in three stages now:
Checks for an exact commit ID match. This is considered an exact
match (mostly useful when matching without changing something that
impacts the commit ID -- this could be used in time to ease attaching
files or publishing changes, for example).
Checks each review request using
SCMClient-provided logic, using
extra_datafor any state it wishes to check. This can
Trueif there's an exact match,
Falseif a review request
should be outright rejected (skipping any future matching logic), or
Noneto proceed to fuzzier matching.
Checks the summary and description against the tree, as before. This
can generate an exact match or a list of fuzzy matches.
If there's only one exact match, then it wins.
If there's more than one exact match, they're considered high-priority
fuzzy matches, appended to any other fuzzy matches found.
This will give us a lot of options down the road for enhancing
operations like posting/updating review requests, closing review
Short-term, SOS is the only tool that utilizes the
matching, and will match a review request if the workarea ID and
changeset ID stored when creating/updating a review request both match
the local workarea.
There are a few deprecations and related changes:
Much of the older match code (the
num_exact_matches()) has been deprecated
and will be removed in RBTools 4.0.
guess_existing_review_requestare deprecated as well, as they were
required for older logic anyway, and all internal uses hard-code these
submit_aswill be required in RBTools 4.0. This function will
require an explicit username and an active session going forward,
to reduce the number of places that can trigger a login flow.
An exception for when
review_requestswas falsy has been removed,
as it was checking if a
ListResourcewas falsy. This never got hit
because resources are (currently) never falsy.
Some comments have been added with planned deprecations to make in
Unit tests were added for non-deprecated functionality.
Unit tests all pass.
Tested this with updating a variety of Git-based changes (which do not
use SCMClient matching) and SOS changes (which do). I haven't encountered
any regressions or noticeable changes in behavior.