4709: Issues should be dropped when a new revision is published

jcannon
4648

What version of Review Bot are you using?

1.0

Which tool(s) does this relate to?

N/A

Describe the enhancement and the motivation for it.

When a new revision is published, the tools re-run and open new issues. If the user hasn't fixed any of the issues brough up on the old revision, the number of issues is now doubled.
Review bot should drop the issues opened on the previous revision as they are no longer valid.

Please provide any additional information below.

jcannon
#1 jcannon

Or deleted? Perhaps it should be a user option?

david
#2 david

Definitely not deleted (keeping accurate history is important). I'd be willing to consider having it close them, especially if there's a toggle for it.

jcannon
#3 jcannon

Would you prefer for the dropping to be done server-side (reviewbotext) or client-side (reviewbot worker).
I see a lot of pros of server-side:
- All changes server-side, absolutely no need to update clients.
- Can update the review directly without going through the API.
- No delay from review posted -> issues dropped.
- Don't have to worry about getting into funny states (if the task fails, does it still drop comments?)

jcannon
#4 jcannon

Review posted: https://reviews.reviewboard.org/r/10061/

misery
#5 misery

Well, strange that my report is not listed anymore here. Ist ist a splat bug? There are multiple reports of me that are hidden.

my report: https://hellosplat.com/s/beanbag/tickets/4648

jcannon
#6 jcannon

Of course the drop should only occurs just before it posts the updates for the issues because an "internal error" of a review bot run would drop all issues without adding new ones.

So one downside to server-side dropping is it is unconditional on tool status.
I could honestly see arguments going either way on that. "Well you posted new code, so the old code isn't relevant anymore" vs. "But the old code had issues, and now I don't know what's wrong with the new code".

The folks at reviewboard can correct me here, but I've been thinking about tool errors as something that should almost never happen. So I'm not as concerned about what happens (within reason) if a tool errors.

david
#8 david

Fixed in Review Bot master (1ccc5f5). This will ship in Review Bot 2.0, coming soon.

  • -New
    +Fixed