Live data from Hacker News

Show HN: Crocodile - Better code review for GitHub

crocodile.dev

51–60 of 62 posts

Re: Show HN: Crocodile - Better code review for GitHub

#51
post #18

This is cool! I'm happy to see more options in this space. The best code review tool I've ever used was a tool at Google called Critique.[0] They've open-sourced it as Gerrit[1], but there are sadly no hosted versions available for under $15k/yr, and it's complicated to self-host. I've been using Reviewable, and my experience has been good not great. Github's native code review has caught up a bit, but Github's revie…

Hey, thanks for the feedback on Reviewable! You make some really good points: > Steep learning curve Guilty as charged. I think it's cohesive and efficient once you learn it but the learning curve is quite steep. We keep trying to think of ways to flatten it a bit but haven't had any great ideas so far. If you have thoughts on this -- or could point us to a great UX designer with dev tools experience -- it would be m…

Thanks for the gracious responses, Piotr! I didn't write that with you in mind, so I hope it doesn't come across as too critical. I have been a happy customer for 7 years.

>>Thread state is unclear

>Huh, I'm surprised by this one...

>Basically, at every step you either reply to keep the discussion going, or click the button to indicate that you're fine with closing it out.

I think there are two problems here:

1) It's not obvious enough that a user is supposed to declare a state after writing a response 2) There are more states than necessary

For (1), the UI flow doesn't hint to the user that they're supposed to do anything after they type their reply. I just tried it and I see this:

https://i.imgur.com/DimycTB.png

It sounds like you're saying the expectation is that users click the circle at the upper right and then choose a state, but that hasn't been obvious to anyone I've introduced to Reviewable.

For (2), this is more personal opinion, but I think comment threads are similar to Github issues in that there only need to be two states: closed or open. The states Reviewable offers to me as the author are:

(1) Discussing, (2) Satisfied, (3) Blocking, (4) Working, (5) Pondering

1, 3, 4, and 5 are all "open" for my purposes, and (2) is closed. It's rare in my experience for a developer to respond halfway through a round of review to say they're still working on or pondering a comment, so I don't need dedicated states for that. They can just keep it open and write a comment like "still working on this one!" I don't see a difference between (1) and (3), as all notes are blocking until they're resolved.

My somewhat controversial opinion is that the author and reviewer should trust each other enough that the author can respond to a note and mark it resolved without awaiting confirmation from the reviewer who wrote the note. It's rare in my experience for an author to incorrectly resolve a note, and when it happens, the reviewer can just reopen it and say, "Hey, I think there was a misunderstanding on this one, and the changes haven't addressed this note." I think forcing the flow into "Discussing" -> "Pending resolution" -> "Confirm resolution" just adds needless friction.

>You shouldn't even need to care about the specific disposition unless you're trying to run more advanced workflows.

We don't, but I see it as a missed opportunity because there's useful information there.

In Critique, it was easy to see at a glance whether a note was active or resolved (active notes had an orange background and resolved notes had a gray background IIRC). In Reviewable, we have to read all the notes more closely to see which are active and which are resolved.

>I'm not a fan of spammy email newsletters so we don't send those.

I don't like those when it's baldly trying to wring more money out of me, but I like it when vendors tell me what's going on and how they're improving the product.

Re: Show HN: Crocodile - Better code review for GitHub

#52

It's worth trying. There's one quibble: How does Crocodile access my source code? Crocodile stores the source code files that are part of reviews to provide a fast user experience. Every file is encrypted with per file data encryption keys. The data encryption keys are then encrypted with a master encryption key. All cryptographic operations are performed using Google Tink, which is a cryptographic library created by…

The master key is stored as a secret in Kubernetes so if someone compromised it, they would be able to decrypt the source code in the reviews.

Re: Show HN: Crocodile - Better code review for GitHub

#53

Nice idea! I'd be interested in giving it a try for our project in the future. It does seem that the demo review is broken in Safari, I get a JS error and the UI doesn't seem to work completely: > [Error] TypeError: e.connect is not a function. (In 'e.connect(l,s)', 'e.connect' is undefined) Also, is there any way to keep up-to-date on the project? I don't see a newsletter or Twitter link on your homepage.

Ah yeah, it only supports Chrome (and Chrome based browsers) and Firefox for now. I've been meaning to add an error message if you use a different browser.

No newsletter yet, but you can follow me on Twitter for now @jameslao.

Re: Show HN: Crocodile - Better code review for GitHub

#56

Earlier quoted context omitted.

> How does the metrics shown in the action help you? We've mostly looked at "Total reviews" and "Total comments". When I put the action in place, I told the team that the goal was for everyone to contribute to the process. I was expecting to look at the stats, make some observations and action items and bring it up during our 1:1. I didn't even need to do that. Everybody took it to heart and contributed in different…

That's actually a use case I've thought about. When review time came around, one of my past managers would compile statistics on everyone's reviews to see how much they've been participating. Another feature I've thought about is "show me all of person X's reviews and comments across all repos". That would have been super useful when I was on promo panels to go through and see if a promo candidate's review comments w…

Exactly! You bring a great point with promo panels where the difference between the amount of time put into building a promo package and the amount of time the panel spend on it can be considerable (my experience at Twitter).

I think when it comes to per-seat pricing, it's good to remember that engineers don't put the credit card in. The person with the credit card will want to know ROI before investing. The engineers will want to try out first as a group and if their experience is positive (some of it might be subjective) then they'll need to build a case for why the company should pay for your product and that's where analytics can be leveraged for sales.

Re: Show HN: Crocodile - Better code review for GitHub

#57
post #51

Earlier quoted context omitted.

Hey, thanks for the feedback on Reviewable! You make some really good points: > Steep learning curve Guilty as charged. I think it's cohesive and efficient once you learn it but the learning curve is quite steep. We keep trying to think of ways to flatten it a bit but haven't had any great ideas so far. If you have thoughts on this -- or could point us to a great UX designer with dev tools experience -- it would be m…

Thanks for the gracious responses, Piotr! I didn't write that with you in mind, so I hope it doesn't come across as too critical. I have been a happy customer for 7 years. >> Thread state is unclear > Huh, I'm surprised by this one... > Basically, at every step you either reply to keep the discussion going, or click the button to indicate that you're fine with closing it out. I think there are two problems here: 1) I…

No worries, feedback of any kind is always welcome, and direct if diplomatic feedback is the best -- which is exactly what you wrote. :) Let me know if you'd like to chat directly at any point, we love talking to our users.

> 1) It's not obvious enough that a user is supposed to declare a state after writing a response

That's because you don't need to declare a state after writing a response. :) The basic workflow is simple: click the button if you're in favor of ending the discussion, otherwise write a response. For more advanced workflows, yes, you can explicitly select a disposition from the dropdown, or prefix your message with a magic keyword to do so automatically.

> For (2), this is more personal opinion, but I think comment threads are similar to Github issues in that there only need to be two states: closed or open.

You're conflating two separate things: each discussion has a state (open -> in progress, closed -> resolved in Reviewable parlance), but so does each participant. The discussion's state is a function of all the participants' states, allowing for a consensus to develop in multi-party reviews without giving any one person the power to unilaterally close the discussion. There are three basic participant states:

Blocking -> prevent discussion from resolving Satisfied -> in favor of resolving discussion Discussing -> neutral; not my monkeys, not my problem

The other two states you mentioned are specialized and affect other features besides the discussion state:

Working -> like Blocking, but also "keeps the ball in your court", so Reviewable knows you're still responsible for further progress on this discussion and won't list it as needing a reply from other participants; it's usually employed by a PR author who agrees with a request but wants to reply with further details before they've actually done the work.

Pondering -> this prevents a draft from being sent when you publish; useful for "notes to self" as you read through the code that may or may not become actual issues you want to raise, and that you don't want to accidentally publish as-is.

It's not a simple system but we figure you've got GitHub for that, so we choose to err on the side of powerful.

> My somewhat controversial opinion is that the author and reviewer should trust each other enough that the author can respond to a note and mark it resolved without awaiting confirmation from the reviewer who wrote the note.

You can do that too! If you initiate a discussion as Discussing, then if the author clicks the Done button (switching to Satisfied) the discussion will automatically be resolved, with no further input from you. You can change your default disposition for discussions created as a reviewer if that's how you prefer to work; see https://docs.reviewable.io/discussions.html#resolution-workf... for details.

> In Critique, it was easy to see at a glance whether a note was active or resolved

In Reviewable, resolved notes are collapsed into the title bar (if just resolved) or into a gutter icon (afterwards), so usually you won't even see them. I'm confused when you say that you can't tell which are active and which are resolved, but perhaps we differ in our understanding of what "resolved" means.

Re: Show HN: Crocodile - Better code review for GitHub

#58
post #18

This is cool! I'm happy to see more options in this space. The best code review tool I've ever used was a tool at Google called Critique.[0] They've open-sourced it as Gerrit[1], but there are sadly no hosted versions available for under $15k/yr, and it's complicated to self-host. I've been using Reviewable, and my experience has been good not great. Github's native code review has caught up a bit, but Github's revie…

Sounds like have some really thoughtful opinions regarding code review tools!

> "Being able to comment on character-level granularity is cool!" This is something that I really like too - I'd go so far as to say PR titles, file names, etc, nearly everything should be commentable.

Have you checked out graphite.dev? Its iterating rapidly and incorporating a lot of what Phabricator and Gerrit did well.

Re: Show HN: Crocodile - Better code review for GitHub

#59
Congratulations on the launch. I find reviewing code on GitHub to be a pain too, so we came up with DiffLens (https://github.com/marketplace/difflens). DiffLens is only concerned with showing better diffs though, and doesn't handle comments on GitHub at all. Maybe there's room here for us to combine our approaches :) Our email is support@difflens.com if you want to get in touch.

Re: Show HN: Crocodile - Better code review for GitHub

#60
post #18

This is cool! I'm happy to see more options in this space. The best code review tool I've ever used was a tool at Google called Critique.[0] They've open-sourced it as Gerrit[1], but there are sadly no hosted versions available for under $15k/yr, and it's complicated to self-host. I've been using Reviewable, and my experience has been good not great. Github's native code review has caught up a bit, but Github's revie…

Sounds like have some really thoughtful opinions regarding code review tools! > "Being able to comment on character-level granularity is cool!" This is something that I really like too - I'd go so far as to say PR titles, file names, etc, nearly everything should be commentable. Have you checked out graphite.dev? Its iterating rapidly and incorporating a lot of what Phabricator and Gerrit did well.

From looking at the website, Graphite seems much more focused on the stacked PRs aspect than reviewing aspect. I'm not as interested in the stacked review aspect, honestly.

Not sure if you've gotten this feedback, but the demos on your website are illegible. They're squeezing a terminal session into a tiny 350px width box:

https://i.imgur.com/UBnNWNT.png

Also, would be good to disclose that you're the co-founder if you're commenting about it publicly.

Post reply on HN