GitHub approvals and completion conditions

Two small updates related to GitHub review approvals:

The sample review completion condition for evaluating GitHub approvals was broken — sorry! I fixed it, but if you used it as a template for your own you'll want to replace all uses of filter with where.

If, on the other hand, you're happy with your LGTM-driven workflow and don't want to use GitHub approvals at all, you can now turn off the "Approve" and "Request changes" options in Reviewable's UI by includingdisableGitHubApprovals: true in the return value of your custom review completion condition.

Mentioned users may not be reviewers

It used to be that if you were @-mentioned in a comment and came to Reviewable, you'd always be treated as a reviewer: many comments would appear as unreplied, files would be looking for your review marks, etc. This could be an overwhelming experience when you just wanted to reply to the one discussion you were mentioned in.

I changed things so that, absent other indicators, users @-mentioned in anything but the top-level discussion are no longer treated as reviewers. Only the discussions they were mentioned in will be unreplied and they will not be asked to review any files. As soon as they take a reviewer-like action, though — such as marking a file reviewed or starting a new discussion — they'll be treated as full reviewers again.

I expect the new logic to still have false positives and negatives but I'm hoping it'll overall work better than before. Please let me know how it goes!

Time-ago dividers positioning

Bowing to convention, I moved the "N days ago" dividers between comments to be located above the corresponding time period, rather than below. This doesn't look quite as good as the previous layout but should allay confusion about when comments were made.

Trust but verify

Do you like using the Discussing disposition to let the PR author resolve a discussion, but wish there was a way to see discussions that were resolved with no further comment? Enter the new "Trust but verify" option:

trustbutverify.PNG

You'll find this toggle in the disposition dropdown, behind a small setting gear icon. If you turn it on, then discussions that get resolved via a disposition change (instead of a comment) and where you are Discussing will show up as unreplied for you so you can double-check. Like so:

newdispositions.PNG

Hopefully this will improve some workflows!

GitHub reviews integration

Integration with GitHub's review approval process is now live! When publishing your drafts you can now choose to comment, approve, or request changes, just like on GitHub:

approve.png

Reviewable will automatically pick the appropriate state based on your discussion dispositions and file review status but you can override it as you see fit. Semantics are exactly the same as on GitHub, though Reviewable will limit you to "Comment" if you lack write permissions to the repo, since other states wouldn't be respected by GitHub anyway.

The state will count towards branch protection required reviews settings as well. Reviewable's default review completion condition remains unchanged and ignores approval states, but there's a new approval-based example in the settings editor if you want to switch to this approach. (But if all you want is the semantics provided by GitHub it's easier to just use the branch protection settings instead.)

The current approval states for all reviewers also show in the review list and the people section on the review page, so you can tell at a glance who's blocking the merge or already approved it (by icon), and whose action is needed next (by color). Note that it's not always the people requesting changes who need to take action next to move the review forward!

Finally, one feature that is unfortunately missing from this release is the publishing of native line-level comments in GitHub rather than rendering them all as part of a single top-level message. It turns out that after more than a year in production, the GitHub reviews API is still missing critical functionality to support this. See issue #437 for details if you're interested. Sorry!

Tweak your contrast

Since day 1, users have been asking me change the contrast on diff highlighting — usually in opposite directions. It's pretty much impossible to pick a value that will satisfy everyone as the actual visual effect is too dependent on monitor quality (and settings) and the users' eyesight. Hence, along with turning animated transitions on or off, you can now also tweak Reviewable's contrast in the account settings dropdown:

contrast.PNG

At the moment this only affects diff highlighting, but as I rework the UI I will try to hook any other partial-contrast styles into the setting as well.

PRs missing from review list

I implemented a workaround for a bug in GitHub's GraphQL queries that sometimes omitted results from large responses. This bug sometimes silently caused random PRs to go missing from the review list. Hopefully the workaround will tide us over until GitHub can fix the root issue...

Improved review status when publishing

The review status inserted into your message when you publish is now the same as the one computed for the commit status message, and also includes the list of people being waited on. If you've set up a custom review completion condition then its output will be used automatically.

Previously, the review status was computed by some special-cased hardcoded logic. It was tricky to update it to use the common code path because the status needs to be computed before the changes being published are committed, to enable recovery from partial failures.

Discussion semantics overhaul

The new streamlined discussion semantics have landed! In summary:

The Discussing disposition becomes simply "I'm not gonna resolve it, but you're welcome to". Resolution is driven solely by dispositions; the invisible state that determined "OK" and "Not OK" is gone. Comment prefixes (such as "FYI") now just change the disposition. Disposition changes and acknowledgements must be published to commit them, even if made without a corresponding comment. A new Working disposition lets you block a discussion and mark it as something you need to come back to. A new Informing disposition lets you start a resolved discussion that will reach everyone.

For more details, please refer to the spec in issue #510. While I did my best to minimize the disruption for reviews in progress, or for reviews accessed by people with a mix of old and new clients, some bumps are inevitable. Old vs new clients will see some differences: you can expect "to reply" counts to change, pending reviewers to be updated, some old dispositions to be adjusted, etc. In principle, the resolution state of discussions shouldn't change, but it's entirely possible I missed some corner cases. My apologies in advance, but trust me, the end goal is worth it!

As always, please eagerly report any bugs or weird state (even if you don't have a perfect repro) and I'll address them ASAP.

Checks and branch protection

GitHub recently introduced the Checks API to allow integrations to report richer information against your commits. Unfortunately, they did so in a way that mixed up legacy commit statuses and the new checks in the branch protection API, leaving OAuth clients no way to disentangle the mess. The likely consequence for you is that if you have Travis CI set up Reviewable recently started showing bogus "missing" checks and refused to let you merge PRs.

I've implemented a heuristic that will allow Reviewable to ignore checks for now, and I'm working with GitHub support to find a better solution. Sorry!

Incidentally, I also fixed Reviewable's treatment of the "include administrators" branch protection flag so it should now be reflected correctly.

No published changelogs yet.

Surely Reviewable will start publishing changelogs very soon.

Check out our other public changelogs: Buffer, Mention, Respond by Buffer, JSFiddle, Olark, Droplr, Piwik Pro, Prott, Ustream, ViralSweep, StartupThreads, Userlike, Unixstickers, Survicate, Envoy, Gmelius, CodeTree