HackerTrans
TopNewTrendsCommentsPastAskShowJobs

nggonzalez

no profile record

Submissions

Show HN: Visibly – More Collaborative and Efficient Code Reviews on GitHub

visibly.dev
57 points·by nggonzalez·قبل 3 سنوات·17 comments

comments

nggonzalez
·قبل 3 سنوات·discuss
Visibly is currently closed source, so I can't provide a link, but curious to hear more about how it may help if some aspects of it were available
nggonzalez
·قبل 3 سنوات·discuss
We haven't gotten to a VSCode extension yet, but it is on the roadmap!

Visibly has some auto transitions built in (ie. auto transitions threads to `addressed` when the diff becomes outdated and auto transitions threads to `in-progress` when the author reacts / comments back) and allows you to change a thread's status via `/v <command>` in comments. These have been a workaround for some of our existing teams, but it is something we'd like to address in the future.
nggonzalez
·قبل 3 سنوات·discuss
> 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.
nggonzalez
·قبل 3 سنوات·discuss
> What makes this not just another tool to be abused by micromanagers?

Visibly is meant to create transparency and visibility for the IC. Today, a lot of managers secretly look at metrics in calibrations and beyond in order gauge productivity and be able to discuss their reports. However, they're seeing a very small piece of the picture and are making apples-to-oranges comparisons. For example, an engineer in a new repo will look way more productive than an engineer in a legacy repo if you look at just lines of code. Visibly's goal is to help make comparisons apples-to-apples and to help you, the IC, see how your tracking in real-time so that you're not surprised later. In addition, Visibly aims to make a more holistic picture of your work so that invisible work that you're doing, like code review, mentorship, design docs, etc, can be captured and used to your benefit.

The intention is to give teams and ICs more knowledge so that they can grow and adapt more quickly and not be surprised by perf at the end.

Moreover, today we don't have a manager portal and have prioritized the IC dashboard. Although it is on the roadmap, managers will see what you see and we are working to make it easy to share highlights and other positives of your performance to them to help avoid this.

I truly understand the concern, but I'm hoping the framing presented and the Visibly team's values around this help ease it for you.
nggonzalez
·قبل 3 سنوات·discuss
> Q) What's the difference between a blocking comment and a "normal" comment + rejecting the PR?

Good question. The granularity of the states are a bit different. Rejecting the PR doesn't capture individual thread states. You may even forget to look at thread later by mistake. With the blocking thread, the individual thread needs to be marked as addressed in order for the status check to go green. This helps ensure that the work gets done and also makes it possible to leave comments like: LGTM after comments or something similar.

Another difference is cultural. At a couple of the companies I was at, Request Changes/Reject on GitHub was viewed as passive aggressive and not used. Blocking and non-blocking comments help these team be more explicit and block on smaller changes in a friendlier way.

> C) Exposing metrics like "Review time per reviewer" may, at the wrong hands, incentivise the wrong behavior. For example, a team lead may view long review times by a particular reviewer as them being slow, whereas they are just more thorough. Tracking the total review time for the PR (aggregating the times from multiple reviewers) is more useful.

Visibly does both, it tracks cumulative time across all reviewers and the individual times. The reasoning is to help show the effort people are putting in to the reviews and the people they are reviewing and to understand, to some degree, if a review was thorough. The metrics help show trends and just surface more knowledge, they should still be a single input in a more holistic picture though.

A few examples that becomes possible with these metrics: (1) You can show why you couldn't get to a task sooner if you spent time reviewing a lot of PRs on the day. (2) You can understand if your PRs "cost" more than other PRs opened by the team. This can help you write smaller PRs or identify the differences between your work and others'.

We also plan to use this time to help surface an estimated "It will take you X min to review this PR" in the future.
nggonzalez
·قبل 3 سنوات·discuss
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
nggonzalez
·قبل 3 سنوات·discuss
Thanks for the thoughtful comment. Sent you a DM
nggonzalez
·قبل 3 سنوات·discuss
Exactly! As we continue to iterate through the visualization side of the metrics, I think this will become even easier to see. Another benefit of the time tracking is that it helps lessen negative emotional responses when receiving a lot of comments - if you see that someone has spent 30 minutes of their time to review, it helps put it in context and show the effort they're putting into you. Without the time, its easy to feel that someone just quickly dropped a bunch of comments and doesn't care