Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Feb 04 04:31
    Client build 5830 deployed
  • Feb 03 22:36
    pkaminski commented #915
  • Feb 03 22:29
    pkaminski labeled #997
  • Feb 03 21:22
    pkaminski labeled #999
  • Feb 03 21:22
    pkaminski opened #999
  • Feb 03 21:18
    pkaminski assigned #995
  • Feb 02 19:00
    pkaminski commented #997
  • Feb 02 19:00
    pkaminski labeled #998
  • Feb 02 16:25
    fahhem labeled #998
  • Feb 02 16:25
    fahhem labeled #998
  • Feb 02 16:25
    fahhem opened #998
  • Feb 02 16:21
    fahhem labeled #997
  • Feb 02 16:21
    fahhem labeled #997
  • Feb 02 16:21
    fahhem opened #997
  • Feb 02 04:11
    Client build 5826 deployed
  • Feb 02 04:02
    pkaminski unassigned #937
  • Feb 02 00:32
    fahhem commented #996
  • Feb 01 22:07
    pkaminski commented #996
  • Feb 01 22:06
    pkaminski assigned #996
  • Feb 01 22:06
    pkaminski labeled #996
Dan Halperin
@dhalperi
Screen Shot 2022-04-05 at 10.39.04 PM.png
(did you mean a different changes box?)
Piotr Kaminski
@pkaminski
Uh, that box. I guess I forgot the conditions under which the link shows up. Try clicking the "show provisional changes" button first, then see if a link appears under it.
Dan Halperin
@dhalperi
oh, yes, that link is the one I clicked to get to the current view.
provisional:
Screen Shot 2022-04-05 at 10.40.38 PM.png
full:
Screen Shot 2022-04-05 at 10.40.58 PM.png
both don't let me see the diff
Piotr Kaminski
@pkaminski
Full is supposed to force all files to show. :/
Dan Halperin
@dhalperi
wait what?
hmm
this time it did
ugh
if I manually select full diff it breaks
Piotr Kaminski
@pkaminski
OK, at least you're unblocked while I go about fixing it.
Dan Halperin
@dhalperi
:+1:
Piotr Kaminski
@pkaminski
Yeah, hiding files is too aggressive after this change -- trying to find a new balance.
Piotr Kaminski
@pkaminski
OK, I pushed a fix: file hiding based on review status is now only on for reviewers, and if all files are hidden there's a new obvious button to help show them all.
Dan Halperin
@dhalperi
confirmed new obvious button on reload and it works
Piotr Kaminski
@pkaminski
Weird, if you're not a reviewer on this PR then it shouldn't be hiding the files at all.
Dan Halperin
@dhalperi
(I did mark some as reviewed after you unblocked me)
Dan Halperin
@dhalperi
Is there a way to deep link to Support announcements? They don't appear to be in the blog. (Aka, if i want to share a link on Slack)
Piotr Kaminski
@pkaminski
You mean from https://headwayapp.co/reviewable-changelog? You can click on a timestamp to get a link to a specific entry.
7 replies
Dan Halperin
@dhalperi
Screen Shot 2022-04-06 at 9.46.47 PM.png
Screen Shot 2022-04-06 at 9.46.51 PM.png
Unless I'm missing it, there's no (entry-specific) direct link in either of those places
Screen Shot 2022-04-06 at 9.49.34 PM.png
As in, wasn't obvious it me that the red bubble took me there.
Piotr Kaminski
@pkaminski
I tried being clever with the red bubble, moving it to the Changelog entry when you dropped down the menu, but it was extremely fragile so I had to give it up. I agree it's not ideal, but given that this way I can offload the entire changelog management, editor, etc. to a third party, I still think it's a net win.
Dan Halperin
@dhalperi
yeah, makes sense.
Michał Kowalczyk
@mkow
I see you already discussed the new hiding of the files above, but what was the rationale behind this change? Now if I manually select the full diff range (base vs newest) I don't get any diffs shown except the commit message. If I manually selected the full diff range, then my intention was to see the full diff ;)
Piotr Kaminski
@pkaminski
Yeah, this needs further work, sorry. Will tweak it today.
Piotr Kaminski
@pkaminski
All right, took a bit longer than I expected but should be fixed. The update logic is 1) if any files in the diff range need reviewing, show only those, but 2) if no files need reviewing, show all of them. (You'll still get the "nothing to diff" message if all diffs are empty.)
Michał Kowalczyk
@mkow
ok, makes more sense, thanks! will see how the new version works in practice :)
Belden Lyman
@belden
I inadvertently renamed our repo, and then I renamed it back. Now old reviews aren't accessible.
20 replies
aselus
@aselus:matrix.org
[m]
hey guys, at a new company again and was trying to get reviewable adopted but hit a wall with the security team. Was wondering if reviewable has any security certs or security documentation?
2 replies
aselus
@aselus:matrix.org
[m]
@pkaminski: thanks, are you guys planning on doing SoC2 at any time in the future? (because I know I will get asked.) I'll check with my security team on the info you passed on though! hopefully can get it resolved w/ the onprem solution
3 replies
Stephen Holsapple
@sholsapp

Hi team, first post here.

I’m trying to enable more and more Github’s branch protections so we have enforcement of some of our policies around code review. I’m having trouble with the Github branch protection described as, “Require conversation resolution before merging”.

For folks using Github's review system, this works fine. However, for folks using Reivewable's review system, it doesn't appear that discussions/comment disposition makes its way into Github's system of record (what I'm calling the place where Github squirrels away all the data under the hood like review status, open discussions).

I think I see what is going on: Reviewable is sync'ing discussions sourced from Reviewable's UI into Github as general comments, as opposed to review comments. The distinction is a general comment does not need acknowledge/resolution from anywhere.

In Github's UI, you create these general comments by navigating to the bottom of a PR and punching in some text to input form there, and push the green "comment" button. In Github's UI, you create a review comment by first navigating to "Start a Review", and then leaving a comment this way.

Has anyone else experienced this and has a solution that would cause Reviewable sourced discussions to show up as Github review comments instead of general comments? I think if you could do this, Github's branch protections would work fine.

2 replies
Dan Halperin
@dhalperi

Reviewable seems like it's taking a LOOOONG time to load a PR. The network seems fine. Is there any good flags to set to debug?

Wondering if it's related to codecov integration that we recently turned on. Is loading that integration blocking the diff loading?

21 replies
Dan Halperin
@dhalperi
Screen Shot 2022-04-25 at 4.13.15 PM.png
Dan Halperin
@dhalperi
Screen Shot 2022-04-25 at 4.23.18 PM.png
Michał Kowalczyk
@mkow
feature idea: what do you think about changing how the "Mark reviewed and go to next file" button works? the problem I have with it is that it goes to the next file with some changes, but jumps over discussions with new comments
optimally I'd like it to jump to the next item which has new contents since the last review
if you like the idea I can file an issue for further discussion ;)
Piotr Kaminski
@pkaminski
So basically a combination of x and .? Dunno, not sure that would fit more people's workflows than the current approach. You can fake it with a compound keyboard shortcut if you want, though, just bind to toggleCurrentFileReviewed(true); nextChangedFileOrUnrepliedDiscussion().
Michał Kowalczyk
@mkow
the problem with using this button is that you sometimes need to manually go back to reply to some discussions
and it's IMO unintuitive that it actually skips some of the updates
so, as the effect, I often resort back to just page up/down
Piotr Kaminski
@pkaminski
Personally, I think pgup/pgdn is a more natural UI anyway, since we do our best not to show anything irrelevant on the page. The "mark and next file" button is more for when you get stuck in single-file mode, in which case skipping over a file isn't as big a deal and you just do a sweep for any remaining discussions at the end.