search

maflcko / DrahtBot

Experimental bot scripts to help maintainers of large open source projects. Also includes some Bitcoin related scripts.
MIT License
10 stars 6 forks source link

Concept (N)ACK issues #33

Open maflcko opened 2 weeks ago

maflcko commented 2 weeks ago

Parsing will never get 100% accuracy, unless everyone is forced to use an exact format.

Here is a collection of parsing issues.

Misdetection

They should be fixed by providing a fresh review, or by downvoting the DrahtBot comment. ("If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.")

Review going stale

Workarounds:

An alternative code fix would be to ignore ACK after an ACK hash. However, this may interfere with the stale-ACK detection, so it should only kick in when ACK hash is not stale.

maflcko commented 2 weeks ago

An alternative code fix would be to ignore ACK after an ACK hash. However, this may interfere with the stale-ACK detection, so it should only kick in when ACK hash is not stale.

Fixed it in https://github.com/maflcko/DrahtBot/commit/cf46398f045566b28748adac92369b5d0383eb86.

This also means that a ACK hash, followed by NACK may or may not be misdetected as ACK.

ryanofsky commented 1 week ago

Thanks for fixing!

This also means that a ACK hash, followed by NACK may or may not be misdetected as ACK.

This doesn't necessarily sound too bad if the PR changed and it shows up a stale ACK. Maybe worse in some other situations, though, but it is hard to be perfect.