Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • May 15 05:16
    Client build 4793 deployed
  • May 14 22:37
    anthonykawa assigned #827
  • May 14 14:23
    Server build 3020 deployed
  • May 14 13:32
    Client build 4789 deployed
  • May 13 12:54
    Client build 4787 deployed
  • May 13 12:05
    Client build 4786 deployed
  • May 13 11:08
    Client build 4785 deployed
  • May 13 10:34
    Client build 4784 deployment failed
  • May 13 09:30
    Server build 3018 deployed
  • May 12 15:17
    Server build 3017 deployed
  • May 12 15:07
    Client build 4780 deployed
  • May 12 04:10
    pkaminski commented #793
  • May 12 04:08
    pkaminski unlabeled #793
  • May 11 15:54
    cgiroux86 assigned #507
  • May 11 15:41
    Server build 3016 deployed
  • May 11 15:38
    Client build 4774 deployed
  • May 11 15:31
    Client build 4773 deployed
  • May 11 14:06
    pkaminski commented #815
  • May 11 13:53
    abarganier commented #815
  • May 11 03:58
    pkaminski commented #815
Dan Halperin
@dhalperi
Confirming: reviewable upstream merges in the default one? Was not aware
Piotr Kaminski
@pkaminski
Yes, it does a merge of the default and the user's results, with priority going to the user. (Only for top-level keys.)
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.)
Dan Halperin
@dhalperi
Am I doing something wrong? The result preview is not working with your suggestion
Screen Shot 2021-05-05 at 9.28.19 AM.png
Piotr Kaminski
@pkaminski
Nope, works for me.
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...
Dan Halperin
@dhalperi
Ahh
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.
Piotr Kaminski
@pkaminski
Huh, that sounds annoyingly plausible. Oops.
Dan Halperin
@dhalperi
I was changing condition, clicking finger, then scrolling down, since I couldn't see condition and result at the same time I didn't realize it was auto-updating.
Piotr Kaminski
@pkaminski
Assuming you're not blocking a discussion and haven't marked any files as reviewed, though, you're most likely getting picked as a candidate reviewer for some file that has no previous reviewers.
Dan Halperin
@dhalperi
I will religiously -reviewer:myself and GH unsubscribe simultaneously, and see if that solves the problem
Circle back :)
Piotr Kaminski
@pkaminski
It should work, in principle... Personally, I've turned off GitHub notifications altogether and instead rely on Reviewable's new webhook feature to pipe review status updates into a shared channel.
Dan Halperin
@dhalperi
Yeah, if I was on GHE and the only repos I contributed to were my company and they all used Reviewable, that might work.
Piotr Kaminski
@pkaminski
Ah, I should've said that I unwatched repositories we have Reviewable set up on, and still have GH comment notifications turned on.
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.
Dan Halperin
@dhalperi
Yeah. GitHub isn't allowed to email me, but I use a Chrome extension that tells me if I have participating isuses/PRs to look at.
Piotr Kaminski
@pkaminski
Oh? What API/logic does it use?
Piotr Kaminski
@pkaminski
Ah, it just hooks into GitHub's web notifications. OK, that's essentially the same as email, but at least the spam is contained to their own page.
Dan Halperin
@dhalperi
The key thing for me is the participating button
That has so far been accurate, unless reviewable adds me back in because I did something wrong :)
Piotr Kaminski
@pkaminski
I guess getting @-mentioned counts as "participating"?
Dan Halperin
@dhalperi
Yes, it brings you back into participating
Piotr Kaminski
@pkaminski
Does it care about whether you're subscribed to the PR?
Dan Halperin
@dhalperi
If you unsubscribe, being @-mentioned resubscribes you
Piotr Kaminski
@pkaminski
So is participating === subscribed?
Dan Halperin
@dhalperi
I'm not sure about that
Piotr Kaminski
@pkaminski
If you were @-mentioned in a PR, which makes you a participant per above, how do you stop being a participant since you can't remove the mention? Is that what unsubscribing does?
1 reply
Dan Halperin
@dhalperi

One q: is there a way to change the "waiting on" definition? We typically use it as an OR, so at this stage:

all files reviewed, all discussions resolved (waiting on @arifogel)

it should not be waiting on anyone.

Piotr Kaminski
@pkaminski
Hmm, assuming that's the default logic's output it's a bit mysterious. Looking over the code I don't see a way for it to produce this result TBH. I really need to get the algorithm to save its reasons along with the output...
Ah, wait, is arifogel the author of that PR?
Dan Halperin
@dhalperi
No
Two people as reviewers on github, one of them reviewed it. arifogel is the other
Piotr Kaminski
@pkaminski
Can you share the state structure for that PR? (email or in private chat here)
David Hartunian
@dhartunian
:wave: hi folks, quick question about file renames in an open PR. we have a PR that just has a single markdown file that's being reviewed. one revision renamed it to a more appropriate file name. Understandably (in hindsight) we lost the connection between comments in r1 when we pushed r2 (containing the new name for the added file). Is there any way to "undo this" and get our comment mapping back? It didn't seem like naming the file back and pushing an r3 was able to fix it.
3 replies
Gilbert Bishop-White
@gilbertbw
image.png
I am seeing what I think is a graphical bug in the file matrix. Not sure if the rounded colour in the cells is intended (and if so what the meaning is?) And the brackets are misaligned.
3 replies
Also a little typo in the context help for comments
1 reply
image.png
Gilbert Bishop-White
@gilbertbw
What does this little clipboard looking icon in a comment preview mean?
26 replies
image.png
Gilbert Bishop-White
@gilbertbw
I'm getting this message on a file that is not binary, is there a way to force it to be treated as text?
3 replies
image.png
Gilbert Bishop-White
@gilbertbw
it it possible to start a discussion on a whole text file, like you can with binary files? Or is the recommended practice to just add the comment to the first line of the file?
Piotr Kaminski
@pkaminski
The latter. See Reviewable/Reviewable#26 for a discussion.