I just tried out simul-unsubscribing and -reviewer on batfish/batfish#6974
I can't remove myself from the reviewers list, oddly.
It does generate its own "pending reviewers" list but this is not communicated to GitHub, and won't cause any GitHub notifications. (It would cause Reviewable notifications but you're probably not using those.)
@pkaminski Do you mean this, which definitely notifies me on github? - every action anyone takes on the review IIUC
The "waiting on" list is indeed what I refer to as "pending reviewers". It's set by the completion condition (or our default version of it). However, by itself, it doesn't interact with GitHub and won't cause notifications (unless you also have
webhook set, but I'm guessing not). That's why I was asking about syncRequestedReviewers and the corresponding toggle under Publish since that will cause Reviewable to sync pending reviewers with GitHub's requested reviewers, leading to notifications. So we should dig into that side, and also figure out why Reviewable keeps putting you on the pending reviewers list (which isn't affected by unsubscription, though it can be impacted by requested reviewers -- there may be a race condition of sorts there since there's a potential dependency cycle).
OK, so the real problem is that you keep ending up in the "waiting on" list. This gets computed in the completion condition -- it looks like you have a custom one, but it's based on some very old example code and AFAICT only adds
disableGitHubApprovals: true to the result. Does that sound right?
I think that perhaps if I
-reviewers:me and then immediately unsubscribe from github issue, it might work. It's just a tetchy pattern to get right.
I'm also thinking I should instrument the default logic so it returns reasons why people end up on the list, and find a way to display those in Reviewable. That would help us figure out why you keep getting put back on -- it might well be a bug in the algorithm.
(And the merging wasn't always the case, I added it in at some point since copying the whole condition to make one tiny change was getting too crazy.)
Maybe just slow? I don't think there's any situation where the evaluation result would remain completely blank. It should either show a result or an error.
But then it should show a tiny spinner... Hmm...
I think the bug is that if you click the "finger down" button to apply PR, without changing the text or the PR #, then it clears the result but doesn't update.
Circle back :)
But it's definitely not ideal and can miss non-review updates. I just got sufficiently annoyed with the GH spam that I decided this was the lesser evil.
That has so far been accurate, unless reviewable adds me back in because I did something wrong :)
1 reply
Ah, wait, is arifogel the author of that PR?
Two people as reviewers on github, one of them reviewed it. arifogel is the other