3 ms·
> Only one person seems to have approved the changes (opetruzel) ... and that's opetruzel's (almost) only contribution to anything in github in his/her two yea
by emmelaich 9y ago
> Only one person seems to have approved the changes (opetruzel)
... and that's opetruzel's (almost) only contribution to anything in github in his/her two year history.
Isn't that odd?
- CydeWeys 9y agoWow, good catch. There are plenty of malicious attackers in the cryptocurrency space, so you should definitely take reviews from not-yet-established contributors with a huge grain of salt. A review of this magnitude should have required review from several well-known contributors. We use a better pattern at work. On our project, we require approval from members within the team for a commit to go in. If there's one person with the best experience in any given area, or even just a strong opinion, then their approval is required as well. It's a big anti-pattern that I catch myself thinking about occasionally. It's temping to send out a commit for review by more junior/inexperienced code members, bypassing the more experienced reviewer who you know will have lots of input you might not necessarily want. That's what Jeff Garzik did here; by the end of it he didn't actually want a review, just a rubber stamp, and so any stamp would do. Thus the entire point of doing reviews was bypassed. It's particularly egregious that he never even bothered to write tests as requested by another reviewer.