[mtlynch]

>See reviewer presence so that you can see if someone is already reviewing and avoid unnecessary pings.

>Visibly values privacy and security above all else

These two things seem squarely at odds. Personally, I don't want my code review tool announcing to developers when I'm looking at their PR.

The fact that it's a Chrome extension would also be a big blocker for me. I run a dev team, and I wouldn't ask any of my teammates to install a browser extension to participate in code reviews, especially one that's only available for Chrome.

I care a lot about code reviews [0], but the features here don't really resonate with me.

- Blocking vs. non-blocking: This is useful, but the default should be that a comment is blocking. Otherwise, it makes the default that all the reviewer's notes are optional unless they go out of their way to mark them as important.

- Thread status history: I think the status history is much less important than the status right now. In other words, I'd like to know if a review is blocked on the author or reviewer. I'm not that interested in what the status was yesterday.

- Metrics: I worry that these focus on the wrong thing. I'm trying to maximize usefulness of my review rather than speedrun through them to improve my PR time.

One of the huge gaps in Github's tool is the ability to do multi-round reviews (e.g., "show me what's changed since my last round of notes"). You can kind of bend Github to show you the right diff, but then it can't show the comments in a sensible way. Two tools that solve this well are CodeApprove[1] and Reviewable[2].

[0] https://mtlynch.io/tags/code-review/

[1] https://codeapprove.com

[2] https://reviewable.io

[nggonzalez]

Thanks for the extensive feedback. Here are my thoughts on these things:

> Personally, I don't want my code review tool announcing to developers when I'm looking at their PR.

This feature is similar to Google docs presence and helps prevent unnecessary work and pings. In my experience, people generally want to know when someone is looking at their PR so they know whether they need to find another reviewer or follow up.

> The fact that it's a Chrome extension would also be a big blocker for me.

Can you expand a little more on this point? At my previous companies, almost all development and debugging was done through Chrome. Happy to expand to other browsers. To provide more context, the reason it is an extension is so that it can update GitHub in place, rather than forcing you to navigate to another website and learn that experience too.

> - Blocking vs. non-blocking: This is useful, but the default should be that a comment is blocking. Otherwise, it makes the default that all the reviewer's notes are optional unless they go out of their way to mark them as important.

The default is blocking. You make something non-blocking by unchecking the box or writing "nit:"

> - Thread status history: I think the status history is much less important than the status right now. In other words, I'd like to know if a review is blocked on the author or reviewer. I'm not that interested in what the status was yesterday.

The current status always visible in the thread header and footer. You can expand the history to see all statuses, but you don't need to do so to see the latest.

> Metrics: I worry that these focus on the wrong thing. I'm trying to maximize usefulness of my review rather than speedrun through them to improve my PR time.

Which aspects of metrics do you feel lead to that incorrect focus? The intention with the review time metrics is definitely not to speedrun, it is quite the opposite - with the review time you can see how much effort has gone into the review, and so it helps naturally push developers to smaller PRs and to also leave more thorough reviews. Without the time, it's really easy to review a 1000 line PR in less than minute because there's no accountability. With the time, it would be easy to spot these cases. This goes back to the classic meme of short PR == 20 comments and large PR === LGTM with no comments

[mtlynch]

>This feature is similar to Google docs presence and helps prevent unnecessary work and pings.

Right, I understand, and I think for developer tasks like reviewing code or reviewing design documents, that's a negative feature in docs.

When I'm reviewing someone's work (be it a design doc or a PR), I don't want them watching me as I do it. I want time to focus privately, and I want to be in control of when I share my feedback with them.

>In my experience, people generally want to know when someone is looking at their PR so they know whether they need to find another reviewer or follow up.

I'm having trouble imagining a scenario where that would be needed.

If it's an urgent PR that needs review ASAP (rare in my experience, but it happens), I'd ping them on another channel to get positive confirmation they're on it and understand it's urgent.

If it's a normal PR, and the norms on my team are that PRs are reviewed in <1 business day, and they're over the limit, and I'm blocked, I'd send them a ping to make sure they're on it. I guess Visibly would prevent me from pinging them if I caught them in the 60-90 min window where they're beyond the normal deadline but still in the middle of a review, but I'd expect those cases to be fairly rare.

What's the typical flow where the presence feature would save a ping?

>Can you expand a little more on this point? At my previous companies, almost all development and debugging was done through Chrome.

Through Chrome specifically or through a web browser? My teammates do a lot of their work in a web browser, but they use a variety of browsers.

I should clarify that I work with a team of freelance developers, so I don't like asking them to install unnecessary software on their systems. I definitely wouldn't ask them to install surveillance software, which it sounds like is what Visibly is.

We currently do code approve with CodeApprove, which everyone on the team likes. We do have to learn new UI flows, but they're pretty simple. It was a problem with Reviewable, which had a steeper learning curve.

>The default is blocking. You make something non-blocking by unchecking the box or writing "nit:"

Ah, okay. I like that behavior.

>The current status always visible in the thread header and footer. You can expand the history to see all statuses, but you don't need to do so to see the latest.

That sounds good, but my feedback was more about highlighting that as a first-class feature of the product. I don't think status history is an important thing to surface, so it might not be a worthwhile thing to highlight as a feature.

>Which aspects of metrics do you feel lead to that incorrect focus? The intention with the review time metrics is definitely not to speedrun, it is quite the opposite - with the review time you can see how much effort has gone into the review, and so it helps naturally push developers to smaller PRs and to also leave more thorough reviews. Without the time, it's really easy to review a 1000 line PR in less than minute because there's no accountability. With the time, it would be easy to spot these cases. This goes back to the classic meme of short PR == 20 comments and large PR === LGTM with no comments

Oh, I thought the PR times were only visible to the reviewer. It sounds like you're saying that the PR times are public because their purpose is to snitch on the reviewer to their teammates and reveal that they didn't spend enough time on the review.

Again, I feel like this is a big contradiction to the promise of "Visibly values privacy" because I definitely don't want a surveillance tool watching me do reviews and reporting my behavior to my teammates.

If the problem is that developers are speeding through reviews and rubber stamping them without really reading them, that's a culture problem and not a tooling problem. Attempts to reduce developer productivity down to simple metrics like "lines of code written" or "minutes per code review" are counterproductive.

[nggonzalez]

> When I'm reviewing someone's work (be it a design doc or a PR), I don't want them watching me as I do it. I want time to focus privately, and I want to be in control of when I share my feedback with them.

That's fair. I'll think about how I can address this longer-term. Would being able to enter a "privacy" mode that hides your presence help?

> What's the typical flow where the presence feature would save a ping?

The typical flow I envision is someone on the same team following up with a coworker after 30-40 minutes after that person has indicated that they will review. It helps them understand if they need to ping again as a reminder. The reason someone may ping in that window is because PRs behave like black boxes after the link is sent - they have no idea if someone is reviewing and how far along that reviewer may be are; the author just has to wait until the comments come.

We're currently working on real notifications pipeline, so this won't be a huge use case in the future.

A different use case where presence may be more useful is to prevent overlapping reviews when pings to codeowners teams are automatic. If you can see someone else is reviewing, then you can defer to them.

> I should clarify that I work with a team of freelance developers, so I don't like asking them to install unnecessary software on their systems. I definitely wouldn't ask them to install surveillance software, which it sounds like is what Visibly is.

I disagree on the surveillance comment - Visibly uses GitHub data that is readily available, augments it with the review time metrics, and allows you to see the progression over time. Beyond that, Visibly does not monitor your work; it does not track what sites you're on or how you do your work. Visibly is only active on github.com and only on PR pages atm. It also takes the most minimal permissions to do this.

Visibly is there to help you and your team understand how much time and effort reviews are taking so you identify trends and better optimize processes. It's also there to help the IC represent their work come perf time - it's really hard to figure out what to write in a perf review when you have to sift through months of work and reviews.

> Oh, I thought the PR times were only visible to the reviewer.

PR times are visible to those who have read/write permissions on the repo. Just like they can see comments / reviewers, they can see status and time. It's just another decision point.

> Attempts to reduce developer productivity down to simple metrics like "lines of code written" or "minutes per code review" are counterproductive.

I agree and not what Visibly is doing. Visibly surfaces metrics to give you more insight. If you don't know how much time reviews are taking, it's hard to accurately estimate timelines, hard to understand the thoroughness of reviews you receive, hard to see the cost of tech debt / complex code, etc. Visibly provides knowledge that you can use to adapt and improve collaboration.