3 ms·
Thanks for the detail! My personal experience is generally in strategy 1 as you outlined, especially in being that lead person. I know a lot of people like the
by davegaeddert 11y ago
Thanks for the detail! My personal experience is generally in strategy 1 as you outlined, especially in being that lead person. I know a lot of people like the appeal of strategy 2 (myself included) but like you mentioned, there's a number of ways that critical code can fall through the cracks. It's great for a general "good work, I like the way you did that" or "what about doing it like this" kind of review but beyond that doesn't provide many safeguards. That being said, I've heard a number of complaints about the organizational/cultural aspect of strategy 1.
In full disclosure, part of the reason I'm asking is to do some research for a product of mine: https://pullapprove.com https://pullapprove.com. Currently I feel as though it serves as a fairly flexible platform that can accommodate either (or both) of the strategies in various ways, and just want to make sure things are headed in the right direction. As you mentioned, the process for choosing reviewers could be handled several different ways and I wonder if there's room for some tools to assist in that. An interesting one to me (which PullApprove doesn't yet support) is automatically choosing reviewers based on git blame information (https://github.com/facebook/mention-bot https://github.com/facebook/mention-bot) -- which could get at your last point.
I feel like there's a ton of value in doing code review that, I personally, drastically underestimated for a long time. I'm interested to hear more about the kinds of problems different groups have with code review and trying to figure out if/where/how a tool like PullApprove can alleviate some of the pain and make it an easier process to adopt.
- piotrkaminski 11y agoCool tool! I run a GitHub code review app myself (https://reviewable.io https://reviewable.io) and have a similar feature built-in to customize the completion condition: https://github.com/Reviewable/Reviewable/wiki/FAQ#can-i-customize-what-makes-a-review-complete https://github.com/Reviewable/Reviewable/wiki/FAQ#can-i-cust.... The main difference is that you took a declarative approach, which I contemplated but discarded after checking with my users and discovering how much variety there was in their approval algorithms. I'm running user scripts on AWS Lambda instead: http://blog.reviewable.io/your-code-my-app-no-worries http://blog.reviewable.io/your-code-my-app-no-worries. I'll be curious to see how your project works out!
- davegaeddert 11y agoAwesome! I actually contemplated going that route as well, but obviously decided not to. Cool use for Lambda though. Do you find that people are using that feature?
- piotrkaminski 11y agoSome people are, but mostly just a hardcore few who are using protected branches on GitHub. I expect usage to tick up as I ramp up (in app) marketing for the feature and move upmarket into larger companies with more formal development workflows. But also, this was an investment in infrastructure that will allow me to offer other deep customizations later on.