Clear status of reviewer for a withdrawn or rejected review

[ietf-svn-bot]

owner:sasha@dashcare.nl resolution_fixed type_defect | by mehmet.ersue@nsn.com

---

If an assigned review gets withdrawn or the reviewer rejects a review the status of the assigned reviewer is not cleared to its original status. The tool behaves as if the review was not withdrawn or the review was not rejected. As a result the reviewer is put to the end of the round robin list and is not provided for a new review. The issue has been discussed with Tero during IETF 100.

---

Issue migrated from trac:2420 at 2022-03-04 06:36:09 +0000

[ietf-svn-bot]

@mehmet.ersue@nsn.com edited the issue description

[ietf-svn-bot]

@sasha@dashcare.nl changed status from new to assigned

[ietf-svn-bot]

@sasha@dashcare.nl changed owner from `` to sasha@dashcare.nl

[ietf-svn-bot]

@sasha@dashcare.nl commented

---

I am wondering what would be the appropriate implementation here (and that should end up in ReviewerQueuePolicy too).

In the SOW for this ticket, it said "If a review assignment is rejected or withdrawn, the reviewer should be returned to the position in the policy that was held before the proposed assignment was made".

Simply pointing NextReviewerInTeam at that reviewer seems rather crude, because it changes the entire order. In theory, we could do exactly that, and then raise skip_next for everyone between the old and new NextReviewerInteam position. But that feels cruddy and I feel skeptical whether it works for all cases.

Currently, I don't think we have a concept of "position in the policy" beyond NextReviewerInTeam and an alphabetical order. A fairly complex option would be to record the entire upcoming order in the database, rather than a pointer to the next person. This would allow arbitrary shuffling, but also make some things more complex - a newly added reviewer has to be inserted into the order at an appropriate point, whereas now they automatically get included, and I expect there will be other complexities.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

Why don't we try just making the reviewer available, and then setting the new "asked for ASAP assignment" bit for them?

[ietf-svn-bot]

@sasha@dashcare.nl commented

---

Yes, that's an easy option and should work quite well. Should there be some cutoff of how long it has been since assignment for this to happen? As in, only set the flag if the assignment was fewer than X weeks/days ago?

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

I don't think so. If someone rejects a review, they're available right now for a review. It would be fair to give them the next available review, even if the rejection is quite stale. They "owed" the team a review since the rejected one was assigned, and that debt should apply now.

[ietf-svn-bot]

@sasha@dashcare.nl changed status from assigned to closed

[ietf-svn-bot]

@sasha@dashcare.nl changed resolution from `` to fixed

[ietf-svn-bot]

@sasha@dashcare.nl commented

---

Fixed in 0c0980c:

Fix #2420 - Add reviewer back to top of the queue after rejected/withdrawn reviews.

[ietf-svn-bot]

@sasha@dashcare.nl commented

---

This was merged to trunk and released in fcb6806.