Reviewable/Reviewable

Support for Reviewable

Dan Halperin

@dhalperi

K. Would a screencap or other info be useful? (As always, I use Chrome latest on MacOS latest, with animations disabled)
Or I can file an issue.

Piotr Kaminski

@pkaminski

Actually, looks like it's just completely broken. Arrgh. I'll get it fixed tonight...

I was getting some weird errors about event.target.closest being undefined that I kludged away, but perhaps broke things doing that...

Piotr Kaminski

@pkaminski

Turns out it was a regression from the last phase of migrating the review page to Vue, that I failed to catch during code review because Reviewable can't yet manage cross-file block moves. :( Should be fixed now.

Dan Halperin

@dhalperi

Seems to be working, thank you. Fast as always!

Alexey Feldgendler

@feldgendler

Hi! In the My Reviews dashboard, where do the unreviewed files counts come from?

I often see small values like 1 there, but when I click a review, it has dozens of unreviewed files.

Piotr Kaminski

@pkaminski

Those are recalculated on the fly, but without all the information available on the actual review page. In practice, they should only differ when the PR has renamed files, and if they do differ they should show "N+" to indicate that there might actually be more. Are you observing something different?

Alexey Feldgendler

@feldgendler

@pkaminski Yes, I often see numbers (without a plus sign) that are smaller than the actual number of unreviewed files.

Piotr Kaminski

@pkaminski

Can you please point to a sample PR? (Ideally not under active review and with not too many files, as it'll make things easier to debug.)

Alexey Feldgendler

@feldgendler

@pkaminski Today I even saw a review that appeared in My Reviews without an unreviewed number at all, and it did have unreviewed files inside.

@pkaminski I can hold a review for you (not touch for a while). Can you ping me when you have the time to investigate? I'll find one and tell everyone not to touch it.

Piotr Kaminski

@pkaminski

@feldgendler If you're around, I have a few hours available starting now-ish.

Piotr Kaminski

@pkaminski

Given our offset timezones, perhaps it would make sense for you to prep a PR at the end of your workday so I could investigate in my morning time.

Marc Zych

@marczych

Are there any settings for not requiring review of files that only have base commit changes? We're getting some review fatigue from developers e.g. open PR, reviewer reviews all files, author merges the target branch, and the author asks the reviewer on slack to rubber stamp the slew of files that only have base commit changes.

I'm seeing baseCommitSha in the completion condition input but I suspect that it can't be used to detect when a file only has base commit changes since it was last reviewed. Any ideas?

4 replies

Alexey Feldgendler

@feldgendler

@pkaminski Alright, try this one: https://reviewable.io/reviews/ridge/tectonic/7685

Piotr Kaminski

@pkaminski

Thanks, could you please add screenshots of what you see on the dashboard and the review page for this PR?

Alexey Feldgendler

@feldgendler

It appears for me in “My Reviews” (“Created by me” section) with the number 3 in the unreviewed files column, but the PR actually has 8 unreviewed files.

Piotr Kaminski

@pkaminski

Just "3" not "3+", right?

Also, can you tell from eyeballing the review which number is accurate (if any)?

Alexey Feldgendler

@feldgendler

@pkaminski Yes, “3” on grey background.

The correct number is 8.

Piotr Kaminski

@pkaminski

OK, I see no review marks for any files/revisions, so that makes sense.

Could you please load the dashboard page with ?debug=model appended and report the value of truss.store.reviews['-MKLW60F3jteR--jiz2L'].files.num?

Piotr Kaminski

@pkaminski

(Particularly interested in estimate, reviewable, unreviewed, and toReview on that object.)

Piotr Kaminski

@pkaminski

@feldgendler ^^

Piotr Kaminski

@pkaminski

So it's suggestive that there are 3 changed files at r2 and all others carried over but I'll need the numbers above to help me trace the causality chain.

Alexey Feldgendler

@feldgendler

@pkaminski Of course, that PR is no longer in this state. I'll try to catch one more and will include the debug info.

Piotr Kaminski

@pkaminski

Note that the review will have a different ID that I'll need to look up for you.

Marc Zych

@marczych

Hi again. I have a PR that has commits that aren't shown in Reviewable despite multiple pushes to try to resync it. Is there a way to force resync a PR?

Also, a different PR didn't get the Reviewable button added to the PR description until I manually navigated to the review. Even then, the first few times displayed "Unable to create review: Request queued but server did not respond" but eventually it worked. I wonder if we're getting rate limited or there is some other issue with our GHE instance. Any ideas?

Piotr Kaminski

@pkaminski

@marczych It sounds like your Reviewable servers might be getting overwhelmed. Probably worth looking at queue health indicators in the logs, and poking around /queues/githubPullRequestSync in the Firebase console to see if any tasks have been attempted a large number of times. (For the review that just won't resync, it's also possible that you've run into a fatal crash driven by the data specific to that PR, so the task can't complete. You should look for "Repeatedly failed to process event" in the logs.)

9 replies

Marc Zych

@marczych

Great, thank you for the pointers!

Alexey Feldgendler

@feldgendler

@pkaminski Remember the issue we discussed above? I caught it again. Here is the debug info you wanted to see: Reviewable/Reviewable#794

Piotr Kaminski

@pkaminski

Thanks, on it!

Alexey Feldgendler

@feldgendler

Is Reviewable down?

Timed out while waiting on cache-osl6528-OSL

On another attempt, got a 502 page with Google's logo.

Alexey Feldgendler

@feldgendler

Seems to be back to normal now.

Piotr Kaminski

@pkaminski

I don't recognize the cache host. Perhaps Firebase's hosting had a hiccup in your region, or some other local cache was interfering...

Piotr Kaminski

@pkaminski

Yep, it was likely this incident: https://status.firebase.google.com/incident/Hosting/20005#20005001

Børge Nordli

@bnordli

All new PRs in our organization are marked as "Archived" in the list. And trying to create the review by going to the usual link spins for a while until it returns "Unable to create review: Request queued but server did not respond"

(Existing reviews work as expected.)

Lukas Vogel

@lukedirtwalker

hello, is there a way to force sync github status with reviewable? For a PR I don't see the github updates in reviewable. but I see comments posted in reviewable in github.

Piotr Kaminski

@pkaminski

@lukedirtwalker Normally loading the review page will force a sync. However, there was an issue with some PRs starting yesterday that made them un-syncable. I fixed it a couple hours ago -- are you still seeing problems? If so, could you send me a sample PR link, either here or to support@reviewable.io? Thanks.

5 replies

@bnordli That's weird, but I think it might be the same issue I just fixed (see above). Is it still happening or are you good now?

Børge Nordli

@bnordli

Still happening, sending link in mail

Piotr Kaminski

@pkaminski

Thanks guys -- I think there's a bigger issue, I'm rolling things back so I have time to figure out what went wrong. Should be working again in a few minutes.

Børge Nordli

@bnordli

Works now, thanks for the quick roll back :)

Lukas Vogel

@lukedirtwalker

Also works for me yes, thanks.

Where communities thrive

People

Repo info

Activity