Add enhanced support for review request matching.
Review Request #12263 — Created April 26, 2022 and submitted
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 requestextra_data
content (if theSCMClient
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
the newSCMClient.get_tree_matches_review_request()
method, which
can checkextra_data
for any state it wishes to check. This can
returnTrue
if there's an exact match,False
if a review request
should be outright rejected (skipping any future matching logic), or
None
to 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
requests, etc.Short-term, SOS is the only tool that utilizes the
extra_data
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
Score
object,
get_possible_matches()
,num_exact_matches()
) has been deprecated
and will be removed in RBTools 4.0. -
The
guess_summary
andguess_description
arguments to
guess_existing_review_request
are deprecated as well, as they were
required for older logic anyway, and all internal uses hard-code these
toTrue
. -
submit_as
will 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_requests
was falsy has been removed,
as it was checking if aListResource
was falsy. This never got hit
because resources are (currently) never falsy.
Some comments have been added with planned deprecations to make in
RBTools 4.0.
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.
Summary | ID |
---|---|
b02ce83aaaff4af4d269e75f2ac22b996240692c |
Description | From | Last Updated |
---|---|---|
F401 'json' imported but unused |
reviewbot | |
F401 'six.moves.urllib.request.urlopen' imported but unused |
reviewbot | |
F401 'rbtools.testing.URLMapTransport' imported but unused |
reviewbot | |
F841 local variable 'transport' is assigned to but never used |
reviewbot | |
F841 local variable 'pending_review_requests' is assigned to but never used |
reviewbot | |
F841 local variable 'transport' is assigned to but never used |
reviewbot | |
F841 local variable 'pending_review_requests' is assigned to but never used |
reviewbot |
- Change Summary:
-
Removed unused imports and variables.
- Description:
-
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_data
content (if theSCMClient
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
the newSCMClient.get_tree_matches_review_request()
method, which
can checkextra_data
for any state it wishes to check. This can
returnTrue
if there's an exact match,False
if a review request
should be outright rejected (skipping any future matching logic), or
None
to 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 requests, etc. Short-term, SOS is the only tool that utilizes the
extra_data
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
Score
object,
get_possible_matches(),
num_exact_matches()`) has been deprecated
and will be removed in RBTools 4.0.
~ -
Much of the older match code (the
Score
object,
get_possible_matches()
,num_exact_matches()
) has been deprecated
and will be removed in RBTools 4.0.
-
The
guess_summary
andguess_description
arguments to
guess_existing_review_request
are deprecated as well, as they were
required for older logic anyway, and all internal uses hard-code these
toTrue
.
-
submit_as
will 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_requests
was falsy has been removed,
as it was checking if aListResource
was falsy. This never got hit
because resources are (currently) never falsy.
Some comments have been added with planned deprecations to make in
RBTools 4.0. Unit tests were added for non-deprecated functionality.
-
- Commits:
-
Summary ID fbe5c2433673f38fc279d934d256e95e2c5c8a54 b02ce83aaaff4af4d269e75f2ac22b996240692c